lkml.org 
[lkml]   [2018]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.18 105/235] ipmi: Rework SMI registration failure
    Date
    4.18-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Corey Minyard <cminyard@mvista.com>

    commit 2512e40e48d21d8bac09f7e91d2c3ceb2d3b50b2 upstream.

    There were certain situations where ipmi_register_smi() would
    return a failure, but the interface would still be registered
    and would need to be unregistered. This is obviously a bad
    design and resulted in an oops in certain failure cases.

    If the interface is started up in ipmi_register_smi(), then
    an error occurs, shut down the interface there so the
    cleanup can be done properly.

    Fix the various smi users, too.

    Signed-off-by: Corey Minyard <cminyard@mvista.com>
    Reported-by: Justin Ernst <justin.ernst@hpe.com>
    Tested-by: Justin Ernst <justin.ernst@hpe.com>
    Cc: Andrew Banman <abanman@hpe.com>
    Cc: Russ Anderson <russ.anderson@hpe.com>
    Cc: <stable@vger.kernel.org> # 4.18.x
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/char/ipmi/ipmi_msghandler.c | 53 ++++++++++++++++++++----------------
    drivers/char/ipmi/ipmi_si_intf.c | 17 ++---------
    drivers/char/ipmi/ipmi_ssif.c | 13 ++------
    3 files changed, 37 insertions(+), 46 deletions(-)

    --- a/drivers/char/ipmi/ipmi_msghandler.c
    +++ b/drivers/char/ipmi/ipmi_msghandler.c
    @@ -3381,39 +3381,45 @@ int ipmi_register_smi(const struct ipmi_

    rv = handlers->start_processing(send_info, intf);
    if (rv)
    - goto out;
    + goto out_err;

    rv = __bmc_get_device_id(intf, NULL, &id, NULL, NULL, i);
    if (rv) {
    dev_err(si_dev, "Unable to get the device id: %d\n", rv);
    - goto out;
    + goto out_err_started;
    }

    mutex_lock(&intf->bmc_reg_mutex);
    rv = __scan_channels(intf, &id);
    mutex_unlock(&intf->bmc_reg_mutex);
    + if (rv)
    + goto out_err_bmc_reg;

    - out:
    - if (rv) {
    - ipmi_bmc_unregister(intf);
    - list_del_rcu(&intf->link);
    - mutex_unlock(&ipmi_interfaces_mutex);
    - synchronize_srcu(&ipmi_interfaces_srcu);
    - cleanup_srcu_struct(&intf->users_srcu);
    - kref_put(&intf->refcount, intf_free);
    - } else {
    - /*
    - * Keep memory order straight for RCU readers. Make
    - * sure everything else is committed to memory before
    - * setting intf_num to mark the interface valid.
    - */
    - smp_wmb();
    - intf->intf_num = i;
    - mutex_unlock(&ipmi_interfaces_mutex);
    + /*
    + * Keep memory order straight for RCU readers. Make
    + * sure everything else is committed to memory before
    + * setting intf_num to mark the interface valid.
    + */
    + smp_wmb();
    + intf->intf_num = i;
    + mutex_unlock(&ipmi_interfaces_mutex);

    - /* After this point the interface is legal to use. */
    - call_smi_watchers(i, intf->si_dev);
    - }
    + /* After this point the interface is legal to use. */
    + call_smi_watchers(i, intf->si_dev);
    +
    + return 0;
    +
    + out_err_bmc_reg:
    + ipmi_bmc_unregister(intf);
    + out_err_started:
    + if (intf->handlers->shutdown)
    + intf->handlers->shutdown(intf->send_info);
    + out_err:
    + list_del_rcu(&intf->link);
    + mutex_unlock(&ipmi_interfaces_mutex);
    + synchronize_srcu(&ipmi_interfaces_srcu);
    + cleanup_srcu_struct(&intf->users_srcu);
    + kref_put(&intf->refcount, intf_free);

    return rv;
    }
    @@ -3504,7 +3510,8 @@ void ipmi_unregister_smi(struct ipmi_smi
    }
    srcu_read_unlock(&intf->users_srcu, index);

    - intf->handlers->shutdown(intf->send_info);
    + if (intf->handlers->shutdown)
    + intf->handlers->shutdown(intf->send_info);

    cleanup_smi_msgs(intf);

    --- a/drivers/char/ipmi/ipmi_si_intf.c
    +++ b/drivers/char/ipmi/ipmi_si_intf.c
    @@ -2083,18 +2083,9 @@ static int try_smi_init(struct smi_info
    si_to_str[new_smi->io.si_type]);

    WARN_ON(new_smi->io.dev->init_name != NULL);
    - kfree(init_name);
    -
    - return 0;
    -
    -out_err:
    - if (new_smi->intf) {
    - ipmi_unregister_smi(new_smi->intf);
    - new_smi->intf = NULL;
    - }

    + out_err:
    kfree(init_name);
    -
    return rv;
    }

    @@ -2227,6 +2218,8 @@ static void shutdown_smi(void *send_info

    kfree(smi_info->si_sm);
    smi_info->si_sm = NULL;
    +
    + smi_info->intf = NULL;
    }

    /*
    @@ -2240,10 +2233,8 @@ static void cleanup_one_si(struct smi_in

    list_del(&smi_info->link);

    - if (smi_info->intf) {
    + if (smi_info->intf)
    ipmi_unregister_smi(smi_info->intf);
    - smi_info->intf = NULL;
    - }

    if (smi_info->pdev) {
    if (smi_info->pdev_registered)
    --- a/drivers/char/ipmi/ipmi_ssif.c
    +++ b/drivers/char/ipmi/ipmi_ssif.c
    @@ -1214,18 +1214,11 @@ static void shutdown_ssif(void *send_inf
    complete(&ssif_info->wake_thread);
    kthread_stop(ssif_info->thread);
    }
    -
    - /*
    - * No message can be outstanding now, we have removed the
    - * upper layer and it permitted us to do so.
    - */
    - kfree(ssif_info);
    }

    static int ssif_remove(struct i2c_client *client)
    {
    struct ssif_info *ssif_info = i2c_get_clientdata(client);
    - struct ipmi_smi *intf;
    struct ssif_addr_info *addr_info;

    if (!ssif_info)
    @@ -1235,9 +1228,7 @@ static int ssif_remove(struct i2c_client
    * After this point, we won't deliver anything asychronously
    * to the message handler. We can unregister ourself.
    */
    - intf = ssif_info->intf;
    - ssif_info->intf = NULL;
    - ipmi_unregister_smi(intf);
    + ipmi_unregister_smi(ssif_info->intf);

    list_for_each_entry(addr_info, &ssif_infos, link) {
    if (addr_info->client == client) {
    @@ -1246,6 +1237,8 @@ static int ssif_remove(struct i2c_client
    }
    }

    + kfree(ssif_info);
    +
    return 0;
    }


    \
     
     \ /
      Last update: 2018-09-24 14:34    [W:4.395 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site