linux-toradex.git/drivers/char/ipmi/ipmi_msghandler.c, branch v4.4.64

ipmi: Remove smi_msg from waiting_rcv_msgs list before handle_one_recv_msg()

2016-07-27T16:47:32+00:00

commit ae4ea9a2460c7fee2ae8feeb4dfe96f5f6c3e562 upstream.

Commit 7ea0ed2b5be8 ("ipmi: Make the message handler easier to use for
SMI interfaces") changed handle_new_recv_msgs() to call handle_one_recv_msg()
for a smi_msg while the smi_msg is still connected to waiting_rcv_msgs list.
That could lead to following list corruption problems:

1) low-level function treats smi_msg as not connected to list

  handle_one_recv_msg() could end up calling smi_send(), which
  assumes the msg is not connected to list.

  For example, the following sequence could corrupt list by
  doing list_add_tail() for the entry still connected to other list.

    handle_new_recv_msgs()
      msg = list_entry(waiting_rcv_msgs)
      handle_one_recv_msg(msg)
        handle_ipmb_get_msg_cmd(msg)
          smi_send(msg)
            spin_lock(xmit_msgs_lock)
            list_add_tail(msg)
            spin_unlock(xmit_msgs_lock)

2) race between multiple handle_new_recv_msgs() instances

  handle_new_recv_msgs() once releases waiting_rcv_msgs_lock before calling
  handle_one_recv_msg() then retakes the lock and list_del() it.

  If others call handle_new_recv_msgs() during the window shown below
  list_del() will be done twice for the same smi_msg.

  handle_new_recv_msgs()
    spin_lock(waiting_rcv_msgs_lock)
    msg = list_entry(waiting_rcv_msgs)
    spin_unlock(waiting_rcv_msgs_lock)
  |
  | handle_one_recv_msg(msg)
  |
    spin_lock(waiting_rcv_msgs_lock)
    list_del(msg)
    spin_unlock(waiting_rcv_msgs_lock)

Fixes: 7ea0ed2b5be8 ("ipmi: Make the message handler easier to use for SMI interfaces")
Signed-off-by: Jun'ichi Nomura 
[Added a comment to describe why this works.]
Signed-off-by: Corey Minyard 
Tested-by: Ye Feng 
Signed-off-by: Greg Kroah-Hartman

ipmi: Don't call receive handler in the panic context

2015-09-03T20:02:29+00:00

Received handlers defined as ipmi_recv_hndl member of struct
ipmi_user_hndl can take a spinlock.  This means that if the kernel
panics while holding the lock, a deadlock may happen on the lock
while flushing queued messages in the panic context.

Calling the receive handler doesn't make much meanings in the panic
context, simply skip it to avoid possible deadlocks.

Signed-off-by: Hidehiro Kawai 
Signed-off-by: Corey Minyard

ipmi: Avoid touching possible corrupted lists in the panic context

2015-09-03T20:02:29+00:00

When processing queued messages in the panic context, IPMI driver
tries to do it without any locking to avoid deadlocks.  However,
this means we can touch a corrupted list if the kernel panicked
while manipulating the list.  Fortunately, current `add-tail and
del-from-head' style implementation won't touch the corrupted part,
but it is inherently risky.

To get rid of the risk, this patch re-initializes the message lists
on panic if the related spinlock has already been acquired.  As the
result, we may lose queued messages, but it's not so painful.
Dropping messages on the received message list is also less
problematic because no one can respond the received messages.

Signed-off-by: Hidehiro Kawai 

Fixed a comment typo.

Signed-off-by: Corey Minyard

ipmi: Don't flush messages in sender() in run-to-completion mode

2015-09-03T20:02:28+00:00

When flushing queued messages in run-to-completion mode,
smi_event_handler() is recursively called.

flush_messages()
 smi_event_handler()
  handle_transaction_done()
   deliver_recv_msg()
    ipmi_smi_msg_received()
     smi_recv_tasklet()
      sender()
       flush_messages()
        smi_event_handler()
         ...

The depth of the recursive call depends on the number of queued
messages, so it can cause a stack overflow if many messages have
been queued.

To solve this problem, this patch removes flush_messages()
from sender()@ipmi_si_intf.c.  Instead, add flush_messages() to
caller side of sender() if needed.  Additionally, to implement this,
add new handler flush_messages to struct ipmi_smi_handlers.

Signed-off-by: Hidehiro Kawai 

Fixed up a comment and some spacing issues.

Signed-off-by: Corey Minyard

ipmi: Remove unneeded set_run_to_completion call

2015-09-03T20:02:27+00:00

send_panic_events() calls intf->handlers->set_run_to_completion(),
but it has already been done in the caller function panic_event().
Remove it from send_panic_events().

Signed-off-by: Hidehiro Kawai 
Signed-off-by: Corey Minyard

ipmi: Make some data const that was only read

2015-09-03T20:02:27+00:00

Several data structures were only used for reading, so make them
const.

Signed-off-by: Corey Minyard

ipmi: Add a comment in how messages are delivered from the lower layer

2015-09-03T20:02:24+00:00

To avoid confusion in the future.

Signed-off-by: Corey Minyard

ipmi: Remove incorrect use of seq_has_overflowed

2015-05-05T19:24:46+00:00

commit d6c5dc18d863 ("ipmi: Remove uses of return value of seq_printf")
incorrectly changed the return value of various proc_show functions
to use seq_has_overflowed().

These functions should return 0 on completion rather than 1/true
on overflow.  1 is the same as #define SEQ_SKIP which would cause
the output to not be emitted (skipped) instead.

This is a logical defect only as the length of these outputs are
all smaller than the initial allocation done by the seq filesystem.

Signed-off-by: Joe Perches 
Signed-off-by: Corey Minyard

ipmi: Remove uses of return value of seq_printf

2015-02-20T02:58:41+00:00

The seq_printf like functions will soon be changed to return void.

Convert these uses to check seq_has_overflowed instead.

Signed-off-by: Joe Perches 
Signed-off-by: Corey Minyard

ipmi: Use is_visible callback for conditional sysfs entries

2015-02-20T02:58:40+00:00

Instead of manual calls of device_create_file() and
device_remove_file(), implement the condition in is_visible callback
for the attribute group and put these entries to the group, too.
This simplifies the code and avoids the possible races.

Signed-off-by: Takashi Iwai 
Signed-off-by: Corey Minyard