All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Nazarewicz <mina86@mina86.com>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [PATCH RESEND] net: wirelesse: wcn36xx: pull allocation outside of critical section
Date: Wed, 11 Dec 2013 17:43:39 +0100	[thread overview]
Message-ID: <xa1tbo0nqqx0.fsf@mina86.com> (raw)
In-Reply-To: <20131211155249.GD7411@tuxdriver.com>

Commit [3469adb3: fix potential NULL pointer dereference] introduced
a check of msg_ind allocation, but omitted allocation of msg_ind->msg.
Moreover, it introduced two if statements, which looked a bit clunky.

This commit moves allocation code outside of the critical section so
there's no need to dance around mutex_unlock, and adds the missing
allocation check.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

On Wed, Dec 11 2013, John W. Linville wrote:
> Missing Signed-off-by...

Sorry about that.

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 8f37562..750626b 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2060,22 +2060,28 @@ static void wcn36xx_smd_rsp_process(struct wcn36xx *wcn, void *buf, size_t len)
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 	case WCN36XX_HAL_MISSED_BEACON_IND:
 	case WCN36XX_HAL_DELETE_STA_CONTEXT_IND:
-		mutex_lock(&wcn->hal_ind_mutex);
 		msg_ind = kmalloc(sizeof(*msg_ind), GFP_KERNEL);
-		if (msg_ind) {
-			msg_ind->msg_len = len;
-			msg_ind->msg = kmalloc(len, GFP_KERNEL);
-			memcpy(msg_ind->msg, buf, len);
-			list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
-			queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
-			wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
+		if (!msg_ind)
+			goto nomem;
+		msg_ind->msg_len = len;
+		msg_ind->msg = kmalloc(len, GFP_KERNEL);
+		if (!msg_ind->msg) {
+			kfree(msg_ind);
+nomem:
+			/*
+			 * FIXME: Do something smarter then just
+			 * printing an error.
+			 */
+			wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
+				    msg_header->msg_type);
+			break;
 		}
+		memcpy(msg_ind->msg, buf, len);
+		mutex_lock(&wcn->hal_ind_mutex);
+		list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
+		queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
 		mutex_unlock(&wcn->hal_ind_mutex);
-		if (msg_ind)
-			break;
-		/* FIXME: Do something smarter then just printing an error. */
-		wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
-			    msg_header->msg_type);
+		wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
 		break;
 	default:
 		wcn36xx_err("SMD_EVENT (%d) not supported\n",
-- 
1.8.4

WARNING: multiple messages have this Message-ID (diff)
From: Michal Nazarewicz <mina86@mina86.com>
To: "John W. Linville" <linville@tuxdriver.com>
Cc: Eugene Krasnikov <k.eugene.e@gmail.com>,
	Peter Stuge <peter@stuge.se>,
	ath9k-devel <ath9k-devel@venema.h4ckr.net>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH RESEND] net: wirelesse: wcn36xx: pull allocation outside of critical section
Date: Wed, 11 Dec 2013 17:43:39 +0100	[thread overview]
Message-ID: <xa1tbo0nqqx0.fsf@mina86.com> (raw)
In-Reply-To: <20131211155249.GD7411@tuxdriver.com>

Commit [3469adb3: fix potential NULL pointer dereference] introduced
a check of msg_ind allocation, but omitted allocation of msg_ind->msg.
Moreover, it introduced two if statements, which looked a bit clunky.

This commit moves allocation code outside of the critical section so
there's no need to dance around mutex_unlock, and adds the missing
allocation check.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/net/wireless/ath/wcn36xx/smd.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

On Wed, Dec 11 2013, John W. Linville wrote:
> Missing Signed-off-by...

Sorry about that.

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 8f37562..750626b 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2060,22 +2060,28 @@ static void wcn36xx_smd_rsp_process(struct wcn36xx *wcn, void *buf, size_t len)
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 	case WCN36XX_HAL_MISSED_BEACON_IND:
 	case WCN36XX_HAL_DELETE_STA_CONTEXT_IND:
-		mutex_lock(&wcn->hal_ind_mutex);
 		msg_ind = kmalloc(sizeof(*msg_ind), GFP_KERNEL);
-		if (msg_ind) {
-			msg_ind->msg_len = len;
-			msg_ind->msg = kmalloc(len, GFP_KERNEL);
-			memcpy(msg_ind->msg, buf, len);
-			list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
-			queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
-			wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
+		if (!msg_ind)
+			goto nomem;
+		msg_ind->msg_len = len;
+		msg_ind->msg = kmalloc(len, GFP_KERNEL);
+		if (!msg_ind->msg) {
+			kfree(msg_ind);
+nomem:
+			/*
+			 * FIXME: Do something smarter then just
+			 * printing an error.
+			 */
+			wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
+				    msg_header->msg_type);
+			break;
 		}
+		memcpy(msg_ind->msg, buf, len);
+		mutex_lock(&wcn->hal_ind_mutex);
+		list_add_tail(&msg_ind->list, &wcn->hal_ind_queue);
+		queue_work(wcn->hal_ind_wq, &wcn->hal_ind_work);
 		mutex_unlock(&wcn->hal_ind_mutex);
-		if (msg_ind)
-			break;
-		/* FIXME: Do something smarter then just printing an error. */
-		wcn36xx_err("Run out of memory while handling SMD_EVENT (%d)\n",
-			    msg_header->msg_type);
+		wcn36xx_dbg(WCN36XX_DBG_HAL, "indication arrived\n");
 		break;
 	default:
 		wcn36xx_err("SMD_EVENT (%d) not supported\n",
-- 
1.8.4


  reply	other threads:[~2013-12-11 16:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-29 17:07 [ath9k-devel] [PATCH] net: wireless: wcn36xx: fix potential NULL pointer dereference Michal Nazarewicz
2013-11-29 17:07 ` Michal Nazarewicz
2013-11-29 17:07 ` Michal Nazarewicz
2013-12-01  0:47 ` [ath9k-devel] " Peter Stuge
2013-12-01  0:47   ` Peter Stuge
2013-12-01  0:47   ` Peter Stuge
2013-12-02 13:09   ` [ath9k-devel] [PATCHv2] " Michal Nazarewicz
2013-12-02 13:09     ` Michal Nazarewicz
2013-12-02 13:09     ` Michal Nazarewicz
2013-12-05  8:41     ` [ath9k-devel] " Eugene Krasnikov
2013-12-05  8:41       ` Eugene Krasnikov
2013-12-05  8:41       ` Eugene Krasnikov
2013-12-06 16:12       ` [ath9k-devel] " John W. Linville
2013-12-06 16:12         ` John W. Linville
2013-12-07 17:13       ` [ath9k-devel] [PATCH] net: wirelesse: wcn36xx: pull allocation outside of critical section Michal Nazarewicz
2013-12-07 17:13         ` Michal Nazarewicz
2013-12-07 17:13         ` Michal Nazarewicz
2013-12-09  7:27         ` [ath9k-devel] " Eugene Krasnikov
2013-12-09  7:27           ` Eugene Krasnikov
2013-12-09  7:27           ` Eugene Krasnikov
2013-12-09 11:34           ` [ath9k-devel] [PATCH] net: wirelesse: wcn36xx: check allocation and trim " Michal Nazarewicz
2013-12-09 11:34             ` Michal Nazarewicz
2013-12-09 11:34             ` Michal Nazarewicz
2013-12-11 15:52             ` [ath9k-devel] " John W. Linville
2013-12-11 15:52               ` John W. Linville
2013-12-11 16:43               ` Michal Nazarewicz [this message]
2013-12-11 16:43                 ` [PATCH RESEND] net: wirelesse: wcn36xx: pull allocation outside of " Michal Nazarewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xa1tbo0nqqx0.fsf@mina86.com \
    --to=mina86@mina86.com \
    --cc=ath9k-devel@lists.ath9k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.