| Message ID | 20200402050219.4842-1-chris@rorvick.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | iwlwifi: actually check allocated conf_tlv pointer | expand |
Chris Rorvick <chris@rorvick.com> writes: > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e > ("iwlwifi: dbg: move debug data to a struct") but does not implement the > check correctly. > > Tweeted-by: @grsecurity > Signed-off-by: Chris Rorvick <chris@rorvick.com> I'll add: Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs") > --- > In this wasn't picked up? Luca, can I take this directly?
On Sun, 2020-04-05 at 11:44 +0300, Kalle Valo wrote: > Chris Rorvick <chris@rorvick.com> writes: > > > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating > > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e > > ("iwlwifi: dbg: move debug data to a struct") but does not implement the > > check correctly. > > > > Tweeted-by: @grsecurity > > Signed-off-by: Chris Rorvick <chris@rorvick.com> > > I'll add: > > Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs") > > > --- > > In this wasn't picked up? > > Luca, can I take this directly? Yes, please take it directly. This can happen in OOM situations and, when it does, we will potentially try to dereference a NULL pointer. Thanks! -- Cheers, Luca.
Luca Coelho <luca@coelho.fi> writes: > On Sun, 2020-04-05 at 11:44 +0300, Kalle Valo wrote: >> Chris Rorvick <chris@rorvick.com> writes: >> >> > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating >> > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e >> > ("iwlwifi: dbg: move debug data to a struct") but does not implement the >> > check correctly. >> > >> > Tweeted-by: @grsecurity >> > Signed-off-by: Chris Rorvick <chris@rorvick.com> >> >> I'll add: >> >> Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs") >> >> > --- >> > In this wasn't picked up? >> >> Luca, can I take this directly? > > Yes, please take it directly. Ok, assigned it to me in patchwork. > This can happen in OOM situations and, when it does, we will > potentially try to dereference a NULL pointer. I'll add this to the commit log.
Chris Rorvick <chris@rorvick.com> wrote: > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e > ("iwlwifi: dbg: move debug data to a struct") but does not implement the > check correctly. > > This can happen in OOM situations and, when it does, we will potentially try to > dereference a NULL pointer. > > Tweeted-by: @grsecurity > Signed-off-by: Chris Rorvick <chris@rorvick.com> Fails to build, please rebase on top of wireless-drivers. drivers/net/wireless/intel/iwlwifi/iwl-drv.c: In function 'iwl_req_fw_callback': drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1470:16: error: 'struct iwl_fw' has no member named 'dbg_conf_tlv' if (!drv->fw.dbg_conf_tlv[i]) ^ make[5]: *** [drivers/net/wireless/intel/iwlwifi/iwl-drv.o] Error 1 make[5]: *** Waiting for unfinished jobs.... make[4]: *** [drivers/net/wireless/intel/iwlwifi] Error 2 make[3]: *** [drivers/net/wireless/intel] Error 2 make[2]: *** [drivers/net/wireless] Error 2 make[1]: *** [drivers/net] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [drivers] Error 2 Patch set to Changes Requested.
On Mon, Apr 6, 2020 at 4:11 PM Kalle Valo <kvalo@codeaurora.org> wrote: > > Chris Rorvick <chris@rorvick.com> wrote: > > > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating > > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e > > ("iwlwifi: dbg: move debug data to a struct") but does not implement the > > check correctly. > > > > This can happen in OOM situations and, when it does, we will potentially try to > > dereference a NULL pointer. > > > > Tweeted-by: @grsecurity > > Signed-off-by: Chris Rorvick <chris@rorvick.com> > > Fails to build, please rebase on top of wireless-drivers. > > drivers/net/wireless/intel/iwlwifi/iwl-drv.c: In function 'iwl_req_fw_callback': > drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1470:16: error: 'struct iwl_fw' has no member named 'dbg_conf_tlv' > if (!drv->fw.dbg_conf_tlv[i]) > ^ > make[5]: *** [drivers/net/wireless/intel/iwlwifi/iwl-drv.o] Error 1 > make[5]: *** Waiting for unfinished jobs.... > make[4]: *** [drivers/net/wireless/intel/iwlwifi] Error 2 > make[3]: *** [drivers/net/wireless/intel] Error 2 > make[2]: *** [drivers/net/wireless] Error 2 > make[1]: *** [drivers/net] Error 2 > make[1]: *** Waiting for unfinished jobs.... > make: *** [drivers] Error 2 > Should be: $ git diff diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index 0481796f75bc..c24350222133 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -1467,7 +1467,7 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context) kmemdup(pieces->dbg_conf_tlv[i], pieces->dbg_conf_tlv_len[i], GFP_KERNEL); - if (!pieces->dbg_conf_tlv[i]) + if (!drv->fw.dbg.conf_tlv[i]) goto out_free_fw; } } "fw.dbg.conf" with a dot not underscore. - Sedat - > Patch set to Changes Requested. > > -- > https://patchwork.kernel.org/patch/11470125/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Mon, Apr 6, 2020 at 9:53 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Mon, Apr 6, 2020 at 4:11 PM Kalle Valo <kvalo@codeaurora.org> wrote: > > > > Chris Rorvick <chris@rorvick.com> wrote: > > > > > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating > > > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e > > > ("iwlwifi: dbg: move debug data to a struct") but does not implement the > > > check correctly. > > > > > > This can happen in OOM situations and, when it does, we will potentially try to > > > dereference a NULL pointer. > > > > > > Tweeted-by: @grsecurity > > > Signed-off-by: Chris Rorvick <chris@rorvick.com> > > > > Fails to build, please rebase on top of wireless-drivers. > > > > drivers/net/wireless/intel/iwlwifi/iwl-drv.c: In function 'iwl_req_fw_callback': > > drivers/net/wireless/intel/iwlwifi/iwl-drv.c:1470:16: error: 'struct iwl_fw' has no member named 'dbg_conf_tlv' > > if (!drv->fw.dbg_conf_tlv[i]) > > ^ > > make[5]: *** [drivers/net/wireless/intel/iwlwifi/iwl-drv.o] Error 1 > > make[5]: *** Waiting for unfinished jobs.... > > make[4]: *** [drivers/net/wireless/intel/iwlwifi] Error 2 > > make[3]: *** [drivers/net/wireless/intel] Error 2 > > make[2]: *** [drivers/net/wireless] Error 2 > > make[1]: *** [drivers/net] Error 2 > > make[1]: *** Waiting for unfinished jobs.... > > make: *** [drivers] Error 2 > > > > Should be: > > $ git diff > diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c > b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c > index 0481796f75bc..c24350222133 100644 > --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c > +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c > @@ -1467,7 +1467,7 @@ static void iwl_req_fw_callback(const struct > firmware *ucode_raw, void *context) > kmemdup(pieces->dbg_conf_tlv[i], > pieces->dbg_conf_tlv_len[i], > GFP_KERNEL); Maybe this diff is clearer: $ diff iwlwifi-actually-check-allocated-conf_tlv-pointer.patch iwlwifi-actually-check-allocated-conf_tlv-pointer-v2-dileks.patch 95a96 > Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs") 99c100,104 < In this wasn't picked up? --- > > [ v1->v2: > - Fix typo s/fw.dbg_conf_tlv/fw.dbg.conf_tlv > - Add Fixes tag as suggested by Kalle > -dileks ] 115c120 < + if (!drv->fw.dbg_conf_tlv[i]) --- > + if (!drv->fw.dbg.conf_tlv[i]) Tested on top of Linux v5.6.3. - Sedat -
On Sun, Apr 5, 2020 at 11:14 AM Kalle Valo <kvalo@codeaurora.org> wrote: > > Luca Coelho <luca@coelho.fi> writes: > > > On Sun, 2020-04-05 at 11:44 +0300, Kalle Valo wrote: > >> Chris Rorvick <chris@rorvick.com> writes: > >> > >> > Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating > >> > conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e > >> > ("iwlwifi: dbg: move debug data to a struct") but does not implement the > >> > check correctly. > >> > > >> > Tweeted-by: @grsecurity > >> > Signed-off-by: Chris Rorvick <chris@rorvick.com> > >> > >> I'll add: > >> > >> Fixes: 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs") > >> > >> > --- > >> > In this wasn't picked up? > >> > >> Luca, can I take this directly? > > > > Yes, please take it directly. > > Ok, assigned it to me in patchwork. > > > This can happen in OOM situations and, when it does, we will > > potentially try to dereference a NULL pointer. > > I'll add this to the commit log. > Hi, Friendly ping. Any progress on this? This patch seems not have landed in Linux v5.7-rc1. $ head -5 Makefile # SPDX-License-Identifier: GPL-2.0 VERSION = 5 PATCHLEVEL = 7 SUBLEVEL = 0 EXTRAVERSION = -rc1 $ LC_ALL=C git apply --check --verbose ../patches/iwlwifi-fixes-5.6/iwlwifi-actually-check-allocated-conf_tlv-pointer-v2-dileks.patch Checking patch drivers/net/wireless/intel/iwlwifi/iwl-drv.c... I have attached my v2 which I have tested on top of Linux v5.6.3. Feel free to add my... Tested-by: Sedat Dilek <sedat.dilek@gmail.com> Regards, - Sedat -
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index ff52e69c1c80..a37f330e7bd4 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -1465,11 +1465,11 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context) if (pieces->dbg_conf_tlv[i]) { drv->fw.dbg.conf_tlv[i] = kmemdup(pieces->dbg_conf_tlv[i], pieces->dbg_conf_tlv_len[i], GFP_KERNEL); - if (!pieces->dbg_conf_tlv[i]) + if (!drv->fw.dbg_conf_tlv[i]) goto out_free_fw; } } memset(&trigger_tlv_sz, 0xff, sizeof(trigger_tlv_sz));
Commit 71bc0334a637 ("iwlwifi: check allocated pointer when allocating conf_tlvs") attempted to fix a typoe introduced by commit 17b809c9b22e ("iwlwifi: dbg: move debug data to a struct") but does not implement the check correctly. Tweeted-by: @grsecurity Signed-off-by: Chris Rorvick <chris@rorvick.com> --- In this wasn't picked up? drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)