netdev - [bug report] net: wwan: iosm: Enable M.2 7360 WWAN card support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <6321a6df-592c-4c2b-939f-25860a97a5ef@stanley.mountain>
Date: Mon, 4 Nov 2024 20:16:53 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: M Chetan Kumar <m.chetan.kumar@...ux.intel.com>
Cc: Johannes Berg <johannes@...solutions.net>, netdev@...r.kernel.org
Subject: [bug report] net: wwan: iosm: Enable M.2 7360 WWAN card support

Hello M Chetan Kumar,

Commit 1f52d7b62285 ("net: wwan: iosm: Enable M.2 7360 WWAN card
support") from Feb 10, 2022 (linux-next), leads to the following
Smatch static checker warning:

	drivers/net/wwan/iosm/iosm_ipc_mux_codec.c:535 ipc_mux_dl_acb_decode()
	warn: potential out of bounds address 'cmdh' user_rl=''

drivers/net/wwan/iosm/iosm_ipc_mux_codec.c
    518 static void ipc_mux_dl_acb_decode(struct iosm_mux *ipc_mux, struct sk_buff *skb)
    519 {
    520         struct mux_acbh *acbh;
    521         struct mux_cmdh *cmdh;
    522         u32 next_cmd_index;
    523         u8 *block;
    524         int size;
    525 
    526         acbh = (struct mux_acbh *)(skb->data);

Smatch marks all skb->data as tainted.

    527         block = (u8 *)(skb->data);
    528 
    529         next_cmd_index = le32_to_cpu(acbh->first_cmd_index);
    530         next_cmd_index = array_index_nospec(next_cmd_index,
    531                                             sizeof(struct mux_cmdh));

How do we know that skb->len is >= sizeof(struct mux_cmdh)?

    532 
    533         while (next_cmd_index != 0) {
    534                 cmdh = (struct mux_cmdh *)&block[next_cmd_index];
--> 535                 next_cmd_index = le32_to_cpu(cmdh->next_cmd_index);

But the most problematic thing is that on the second iteration there is no
bounds checking on next_cmd_index.

    536                 if (ipc_mux_dl_cmdresps_decode_process(ipc_mux, cmdh->param,
    537                                                        cmdh->command_type,
    538                                                        cmdh->if_id,
    539                                                        cmdh->transaction_id)) {
    540                         size = offsetof(struct mux_cmdh, param) +
    541                                 sizeof(cmdh->param.flow_ctl);
    542                         ipc_mux_dl_acbcmd_decode(ipc_mux, cmdh, size);
    543                 }
    544         }
    545 }

regards,
dan carpenter