From patchwork Wed Aug 22 17:20:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corey Minyard X-Patchwork-Id: 144849 Delivered-To: patch@linaro.org Received: by 2002:a2e:164a:0:0:0:0:0 with SMTP id 10-v6csp253881ljw; Wed, 22 Aug 2018 10:20:38 -0700 (PDT) X-Google-Smtp-Source: AA+uWPwKtcROuL1VbsPXgqa4v71pOSt+z1hrRfpOUb4/Rz8rQtNVPbWAS9oB2EN8C4/1eTTkNH8o X-Received: by 2002:a17:902:24e1:: with SMTP id l30-v6mr23544293plg.315.1534958438707; Wed, 22 Aug 2018 10:20:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1534958438; cv=none; d=google.com; s=arc-20160816; b=y9FpZwkJW9u4r9NIJMYrslMR2koOBAaEYe3np7x8MzZFNb2P0rilSuX6FNcuFlrCtT jcW9w27M3553/4q+bzQhCiU/TsdfS1ItDSBvdsW8daEGSGqcy6dFByLbzhWvNrVECe5Y ANp7rd+nbFoILvyOkjPj1nn4jC+7uJXklCCWIy8HYQXAiPv96Uzj1NmDq+AD6fEcJtkg KKXAAi+jNzHd7OiUh5I+CscS9xsCrNv3CAlS0OLjh2h6jS3hhve3TVL1ELGbD7wWnElq cBXRL+xpHFfFCctOdikCV6KAP+75EVQPn4awwDGe7TQxmmw+vIpy18RtdhFneUhNS6NX BCVQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dkim-signature:arc-authentication-results; bh=3Tp7vypN8PuZv1fhf6/fVHodnM+37bTpx8LMX/r5oug=; b=KvEgxARV+v/avFBFdL/f3yoL4eChOxzc0w4AviGEJj5VSslzzBklxJRkVJ13VVk87t Sg3C6by2NirKF0OvUeJdgF2oqXEIwNzV1yOfRn7aNwLLaUG4S1uPlDRHqoH0x/C2L/iN e/r+TVnqUUpO4RqAMo0y0BdclDdN0bbl+6PZT+FmglZbrrdeWDa1rNxzr8qDzNwPLdOf V/430HY1O76i3FkFnnF+yRgfttQTaRTkrkB2pMVGyGQK3wup7xiC8GexHPcvvfscBDwM YzeC1vr7Y/ttP/3BzS3EBtbtZv2BTrhvUkZBPP7CQQ2iJkoHeCWExSpX2IN0smCWxtrf tXTw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b="XU/JgoJx"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id x81-v6si2170671pgx.156.2018.08.22.10.20.38; Wed, 22 Aug 2018 10:20:38 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b="XU/JgoJx"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727452AbeHVUqT (ORCPT + 32 others); Wed, 22 Aug 2018 16:46:19 -0400 Received: from mail-oi0-f67.google.com ([209.85.218.67]:44074 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726876AbeHVUqT (ORCPT ); Wed, 22 Aug 2018 16:46:19 -0400 Received: by mail-oi0-f67.google.com with SMTP id s198-v6so4430467oih.11 for ; Wed, 22 Aug 2018 10:20:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id; bh=3Tp7vypN8PuZv1fhf6/fVHodnM+37bTpx8LMX/r5oug=; b=XU/JgoJxXnFhkmsDpKuVdXC+wqHlyItkQUKY3o4NHGXUXYn0T6lxSgj9qjMco1lG/E 37hcykOdn5xM7Fawq/aWrj0bxruiX4bCLpAqYHFTvdpxtn3oUKD7UXdzimdRtCQo7Kjr ylVV06gQpfFEIFhMiJiY/YTUi9idQyWavSBNRkJ+bck5V64xXaW0K67CT3QhMQX3mEgy O9j++SvG0nMTTH40TrvWJ59UNU8otDnUepuAao7s4MIBPoQcF30I1xDaOOhjSJT1h0WS EKzf5KZ3Q16wrzzc+tFWIzJWDhrWIlgqFvf5URS1SiSFDH844zxBxan6cDqCli/x3X6b E7Eg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id; bh=3Tp7vypN8PuZv1fhf6/fVHodnM+37bTpx8LMX/r5oug=; b=cQ0no507Bech8pEGaCyFVtxl1u6v8GaL9xN1VYpMICKg+lIZ4SCHAyc8xV5qswNWqm hHA4L/jePBFb8Jgwk3xbaZobiUElCJpr54rM04/vIRhaB98E6jAcYITZDYUeezYYZywo pAUhTvlF0kcZNzm29nKWI4HqPfuF2myqcandqMhU/kjtGXck3EsCunoMyACdVp42CSZo 2S9cCrhB4rIEmKYkdnkUSZmnDfYAIDRbAAZEMMOK+XDZ7TAXZpFa0wThbBzfBWo5FtAc VV9RbVC0yTeHEzT6+bwAsuCe9xbWpy/uOCMnMTJCdJ5HSBflS6HO5457KlGt7/laburo doNA== X-Gm-Message-State: APzg51C0kQZR0x0QFJ8Kf+OjikaSEVTmz7h8yzuQQ58EJBMAu4Kybk4y eqf1WL1aL333lBXHMZ6xEA== X-Received: by 2002:aca:c484:: with SMTP id u126-v6mr5114054oif.209.1534958433688; Wed, 22 Aug 2018 10:20:33 -0700 (PDT) Received: from serve.minyard.net ([47.184.170.128]) by smtp.gmail.com with ESMTPSA id s145-v6sm3924581oih.16.2018.08.22.10.20.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Aug 2018 10:20:32 -0700 (PDT) Received: from t430.minyard.net (t430m.minyard.net [192.168.27.3]) by serve.minyard.net (Postfix) with ESMTPA id 1218B776; Wed, 22 Aug 2018 12:20:32 -0500 (CDT) Received: by t430.minyard.net (Postfix, from userid 1000) id 41C503000CA; Wed, 22 Aug 2018 12:20:31 -0500 (CDT) From: minyard@acm.org To: Justin Ernst Cc: Corey Minyard , Andrew Banman , Russ Anderson , openipmi-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: [PATCH] ipmi: Rework SMI registration failure Date: Wed, 22 Aug 2018 12:20:12 -0500 Message-Id: <1534958412-27199-1-git-send-email-minyard@acm.org> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Corey Minyard 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 Reported-by: Justin Ernst Cc: Andrew Banman Cc: Russ Anderson Cc: openipmi-developer@lists.sourceforge.net Cc: linux-kernel@vger.kernel.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(-) -- 2.7.4 Tested-by: Justin Ernst diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 46ab1ba..04f64c5 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -3378,39 +3378,45 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, 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; } @@ -3501,7 +3507,8 @@ void ipmi_unregister_smi(struct ipmi_smi *intf) } 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); diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 2194b4c..bb7d8af 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -2102,18 +2102,9 @@ static int try_smi_init(struct smi_info *new_smi) 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; } @@ -2246,6 +2237,8 @@ static void shutdown_smi(void *send_info) kfree(smi_info->si_sm); smi_info->si_sm = NULL; + + smi_info->intf = NULL; } /* @@ -2259,10 +2252,8 @@ static void cleanup_one_si(struct smi_info *smi_info) 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) diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index fddf646..3574322 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1218,18 +1218,11 @@ static void shutdown_ssif(void *send_info) 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) @@ -1239,9 +1232,7 @@ static int ssif_remove(struct i2c_client *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) { @@ -1250,6 +1241,8 @@ static int ssif_remove(struct i2c_client *client) } } + kfree(ssif_info); + return 0; }