* [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer @ 2013-04-16 13:38 Stanislaw Gruszka 2013-04-16 13:40 ` [PATCH 2/2] iwlwifi: remove redundant argument from iwl_dump_nic_event_log Stanislaw Gruszka 2013-04-16 18:40 ` [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer Jonas Gorski 0 siblings, 2 replies; 7+ messages in thread From: Stanislaw Gruszka @ 2013-04-16 13:38 UTC (permalink / raw) To: Johannes Berg; +Cc: ilw, linux-wireless If on iwl_dump_nic_event_log() error occurs before that function initialize buf, we process uninitiated pointer in iwl_dbgfs_log_event_read() and can hit "BUG at mm/slub.c:3409" Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=951241 Reported-by: ian.odette@eprize.com Cc: stable@vger.kernel.org Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> --- Patch is only compile tested, but I'm sure it fixes the problem. drivers/net/wireless/iwlwifi/dvm/debugfs.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/dvm/debugfs.c b/drivers/net/wireless/iwlwifi/dvm/debugfs.c index 7b8178b..cb6dd58 100644 --- a/drivers/net/wireless/iwlwifi/dvm/debugfs.c +++ b/drivers/net/wireless/iwlwifi/dvm/debugfs.c @@ -2237,15 +2237,15 @@ static ssize_t iwl_dbgfs_log_event_read(struct file *file, size_t count, loff_t *ppos) { struct iwl_priv *priv = file->private_data; - char *buf; - int pos = 0; - ssize_t ret = -ENOMEM; + char *buf = NULL; + ssize_t ret; - ret = pos = iwl_dump_nic_event_log(priv, true, &buf, true); - if (buf) { - ret = simple_read_from_buffer(user_buf, count, ppos, buf, pos); - kfree(buf); - } + ret = iwl_dump_nic_event_log(priv, true, &buf, true); + if (ret < 0) + goto err; + ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); +err: + kfree(buf); return ret; } -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] iwlwifi: remove redundant argument from iwl_dump_nic_event_log 2013-04-16 13:38 [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer Stanislaw Gruszka @ 2013-04-16 13:40 ` Stanislaw Gruszka 2013-04-17 6:23 ` [PATCH 3/2] iwlwifi: remove unneeded goto from iwl_dbgfs_log_event_read Stanislaw Gruszka 2013-04-16 18:40 ` [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer Jonas Gorski 1 sibling, 1 reply; 7+ messages in thread From: Stanislaw Gruszka @ 2013-04-16 13:40 UTC (permalink / raw) To: Johannes Berg; +Cc: ilw, linux-wireless We can check buf against NULL instead of having additional bool variable. Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> --- drivers/net/wireless/iwlwifi/dvm/agn.h | 2 +- drivers/net/wireless/iwlwifi/dvm/debugfs.c | 4 ++-- drivers/net/wireless/iwlwifi/dvm/main.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/dvm/agn.h b/drivers/net/wireless/iwlwifi/dvm/agn.h index e575b9b..48545ab 100644 --- a/drivers/net/wireless/iwlwifi/dvm/agn.h +++ b/drivers/net/wireless/iwlwifi/dvm/agn.h @@ -172,7 +172,7 @@ int iwl_calib_set(struct iwl_priv *priv, const struct iwl_calib_hdr *cmd, int len); void iwl_calib_free_results(struct iwl_priv *priv); int iwl_dump_nic_event_log(struct iwl_priv *priv, bool full_log, - char **buf, bool display); + char **buf); int iwlagn_hw_valid_rtc_data_addr(u32 addr); /* lib */ diff --git a/drivers/net/wireless/iwlwifi/dvm/debugfs.c b/drivers/net/wireless/iwlwifi/dvm/debugfs.c index cb6dd58..17f04de 100644 --- a/drivers/net/wireless/iwlwifi/dvm/debugfs.c +++ b/drivers/net/wireless/iwlwifi/dvm/debugfs.c @@ -2240,7 +2240,7 @@ static ssize_t iwl_dbgfs_log_event_read(struct file *file, char *buf = NULL; ssize_t ret; - ret = iwl_dump_nic_event_log(priv, true, &buf, true); + ret = iwl_dump_nic_event_log(priv, true, &buf); if (ret < 0) goto err; ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); @@ -2269,7 +2269,7 @@ static ssize_t iwl_dbgfs_log_event_write(struct file *file, if (sscanf(buf, "%d", &event_log_flag) != 1) return -EFAULT; if (event_log_flag == 1) - iwl_dump_nic_event_log(priv, true, NULL, false); + iwl_dump_nic_event_log(priv, true, NULL); return count; } diff --git a/drivers/net/wireless/iwlwifi/dvm/main.c b/drivers/net/wireless/iwlwifi/dvm/main.c index b9e3517..74d7572 100644 --- a/drivers/net/wireless/iwlwifi/dvm/main.c +++ b/drivers/net/wireless/iwlwifi/dvm/main.c @@ -1795,7 +1795,7 @@ static int iwl_print_last_event_logs(struct iwl_priv *priv, u32 capacity, #define DEFAULT_DUMP_EVENT_LOG_ENTRIES (20) int iwl_dump_nic_event_log(struct iwl_priv *priv, bool full_log, - char **buf, bool display) + char **buf) { u32 base; /* SRAM byte address of event log header */ u32 capacity; /* event log capacity in # entries */ @@ -1866,7 +1866,7 @@ int iwl_dump_nic_event_log(struct iwl_priv *priv, bool full_log, size); #ifdef CONFIG_IWLWIFI_DEBUG - if (display) { + if (buf) { if (full_log) bufsz = capacity * 48; else @@ -1962,7 +1962,7 @@ static void iwl_nic_error(struct iwl_op_mode *op_mode) priv->fw->fw_version); iwl_dump_nic_error_log(priv); - iwl_dump_nic_event_log(priv, false, NULL, false); + iwl_dump_nic_event_log(priv, false, NULL); iwlagn_fw_error(priv, false); } -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/2] iwlwifi: remove unneeded goto from iwl_dbgfs_log_event_read 2013-04-16 13:40 ` [PATCH 2/2] iwlwifi: remove redundant argument from iwl_dump_nic_event_log Stanislaw Gruszka @ 2013-04-17 6:23 ` Stanislaw Gruszka 2013-04-17 7:10 ` Julian Calaby 0 siblings, 1 reply; 7+ messages in thread From: Stanislaw Gruszka @ 2013-04-17 6:23 UTC (permalink / raw) To: Johannes Berg; +Cc: ilw, linux-wireless, Jonas Gorski Make code simpler a bit. Reported-by: Jonas Gorski <jogo@openwrt.org> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> --- drivers/net/wireless/iwlwifi/dvm/debugfs.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/dvm/debugfs.c b/drivers/net/wireless/iwlwifi/dvm/debugfs.c index 17f04de..d532948 100644 --- a/drivers/net/wireless/iwlwifi/dvm/debugfs.c +++ b/drivers/net/wireless/iwlwifi/dvm/debugfs.c @@ -2241,10 +2241,8 @@ static ssize_t iwl_dbgfs_log_event_read(struct file *file, ssize_t ret; ret = iwl_dump_nic_event_log(priv, true, &buf); - if (ret < 0) - goto err; - ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); -err: + if (ret > 0) + ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); kfree(buf); return ret; } -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/2] iwlwifi: remove unneeded goto from iwl_dbgfs_log_event_read 2013-04-17 6:23 ` [PATCH 3/2] iwlwifi: remove unneeded goto from iwl_dbgfs_log_event_read Stanislaw Gruszka @ 2013-04-17 7:10 ` Julian Calaby 2013-04-17 7:12 ` Johannes Berg 0 siblings, 1 reply; 7+ messages in thread From: Julian Calaby @ 2013-04-17 7:10 UTC (permalink / raw) To: Stanislaw Gruszka Cc: Johannes Berg, Intel Linux Wireless, linux-wireless, Jonas Gorski Hi Stanislaw, On Wed, Apr 17, 2013 at 4:23 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote: > Make code simpler a bit. > > Reported-by: Jonas Gorski <jogo@openwrt.org> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > --- > drivers/net/wireless/iwlwifi/dvm/debugfs.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/iwlwifi/dvm/debugfs.c b/drivers/net/wireless/iwlwifi/dvm/debugfs.c > index 17f04de..d532948 100644 > --- a/drivers/net/wireless/iwlwifi/dvm/debugfs.c > +++ b/drivers/net/wireless/iwlwifi/dvm/debugfs.c > @@ -2241,10 +2241,8 @@ static ssize_t iwl_dbgfs_log_event_read(struct file *file, > ssize_t ret; > > ret = iwl_dump_nic_event_log(priv, true, &buf); > - if (ret < 0) > - goto err; > - ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); > -err: > + if (ret > 0) > + ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); That's not the same, what happens if ret == 0? Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ .Plan: http://sites.google.com/site/juliancalaby/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/2] iwlwifi: remove unneeded goto from iwl_dbgfs_log_event_read 2013-04-17 7:10 ` Julian Calaby @ 2013-04-17 7:12 ` Johannes Berg 0 siblings, 0 replies; 7+ messages in thread From: Johannes Berg @ 2013-04-17 7:12 UTC (permalink / raw) To: Julian Calaby Cc: Stanislaw Gruszka, Intel Linux Wireless, linux-wireless, Jonas Gorski On Wed, 2013-04-17 at 17:10 +1000, Julian Calaby wrote: > Hi Stanislaw, > > On Wed, Apr 17, 2013 at 4:23 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote: > > Make code simpler a bit. > > > > Reported-by: Jonas Gorski <jogo@openwrt.org> > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > > --- > > drivers/net/wireless/iwlwifi/dvm/debugfs.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/wireless/iwlwifi/dvm/debugfs.c b/drivers/net/wireless/iwlwifi/dvm/debugfs.c > > index 17f04de..d532948 100644 > > --- a/drivers/net/wireless/iwlwifi/dvm/debugfs.c > > +++ b/drivers/net/wireless/iwlwifi/dvm/debugfs.c > > @@ -2241,10 +2241,8 @@ static ssize_t iwl_dbgfs_log_event_read(struct file *file, > > ssize_t ret; > > > > ret = iwl_dump_nic_event_log(priv, true, &buf); > > - if (ret < 0) > > - goto err; > > - ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); > > -err: > > + if (ret > 0) > > + ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); > > That's not the same, what happens if ret == 0? It's the same: nothing will be read regardless of whether you call the function or not. johannes ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer 2013-04-16 13:38 [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer Stanislaw Gruszka 2013-04-16 13:40 ` [PATCH 2/2] iwlwifi: remove redundant argument from iwl_dump_nic_event_log Stanislaw Gruszka @ 2013-04-16 18:40 ` Jonas Gorski 2013-04-17 6:17 ` Stanislaw Gruszka 1 sibling, 1 reply; 7+ messages in thread From: Jonas Gorski @ 2013-04-16 18:40 UTC (permalink / raw) To: Stanislaw Gruszka; +Cc: Johannes Berg, ilw, linux-wireless On 16 April 2013 15:38, Stanislaw Gruszka <sgruszka@redhat.com> wrote: > If on iwl_dump_nic_event_log() error occurs before that function > initialize buf, we process uninitiated pointer in > iwl_dbgfs_log_event_read() and can hit "BUG at mm/slub.c:3409" > > Resolves: > https://bugzilla.redhat.com/show_bug.cgi?id=951241 > > Reported-by: ian.odette@eprize.com > Cc: stable@vger.kernel.org > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > --- > Patch is only compile tested, but I'm sure it fixes the problem. > > drivers/net/wireless/iwlwifi/dvm/debugfs.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/wireless/iwlwifi/dvm/debugfs.c b/drivers/net/wireless/iwlwifi/dvm/debugfs.c > index 7b8178b..cb6dd58 100644 > --- a/drivers/net/wireless/iwlwifi/dvm/debugfs.c > +++ b/drivers/net/wireless/iwlwifi/dvm/debugfs.c > @@ -2237,15 +2237,15 @@ static ssize_t iwl_dbgfs_log_event_read(struct file *file, > size_t count, loff_t *ppos) > { > struct iwl_priv *priv = file->private_data; > - char *buf; > - int pos = 0; > - ssize_t ret = -ENOMEM; > + char *buf = NULL; > + ssize_t ret; > > - ret = pos = iwl_dump_nic_event_log(priv, true, &buf, true); > - if (buf) { > - ret = simple_read_from_buffer(user_buf, count, ppos, buf, pos); > - kfree(buf); > - } > + ret = iwl_dump_nic_event_log(priv, true, &buf, true); > + if (ret < 0) > + goto err; > + ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); > +err: Not every error check needs a goto, you can avoid it by inverting the condition: ;-) ret = iwl_dump_nic_event_log(priv, true, &buf, true); if (ret >= 0) /* or maybe even > 0, because AFAICT 0 => nothing to read */ ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); kfree(buf); Jonas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer 2013-04-16 18:40 ` [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer Jonas Gorski @ 2013-04-17 6:17 ` Stanislaw Gruszka 0 siblings, 0 replies; 7+ messages in thread From: Stanislaw Gruszka @ 2013-04-17 6:17 UTC (permalink / raw) To: Jonas Gorski; +Cc: Johannes Berg, ilw, linux-wireless On Tue, Apr 16, 2013 at 08:40:30PM +0200, Jonas Gorski wrote: > > + ret = iwl_dump_nic_event_log(priv, true, &buf, true); > > + if (ret < 0) > > + goto err; > > + ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); > > +err: > > Not every error check needs a goto, you can avoid it by inverting the > condition: ;-) > > ret = iwl_dump_nic_event_log(priv, true, &buf, true); > if (ret >= 0) /* or maybe even > 0, because AFAICT 0 => nothing to read */ > ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret); > kfree(buf); Make sense. Since this is only code cleanup issue and simple_read_from_buffer(..., 0) is ok, I'll post incremental patch which fix this. Thanks Stanislaw ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-04-17 7:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-16 13:38 [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer Stanislaw Gruszka 2013-04-16 13:40 ` [PATCH 2/2] iwlwifi: remove redundant argument from iwl_dump_nic_event_log Stanislaw Gruszka 2013-04-17 6:23 ` [PATCH 3/2] iwlwifi: remove unneeded goto from iwl_dbgfs_log_event_read Stanislaw Gruszka 2013-04-17 7:10 ` Julian Calaby 2013-04-17 7:12 ` Johannes Berg 2013-04-16 18:40 ` [PATCH 1/2] iwlwifi: fix freeing uninitialized pointer Jonas Gorski 2013-04-17 6:17 ` Stanislaw Gruszka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).