From: "Jérôme Pouiller" <Jerome.Pouiller@silabs.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"David S . Miller" <davem@davemloft.net>,
Kalle Valo <kvalo@codeaurora.org>
Subject: Re: [PATCH 08/32] staging: wfx: simplify hif_handle_tx_data()
Date: Thu, 2 Apr 2020 14:44:27 +0000 [thread overview]
Message-ID: <2302785.6C7ODC2LYm@pc-42> (raw)
In-Reply-To: <20200402131338.GS2001@kadam>
On Thursday 2 April 2020 15:13:39 CEST Dan Carpenter wrote:
> On Wed, Apr 01, 2020 at 01:03:41PM +0200, Jerome Pouiller wrote:
[...]
> This is on the TX side so it's probably okay, but one problem I have
> noticed is that we do this on the RX side as well with checking that
>
> if (skb->len < sizeof(struct hif_msg))
> return -EINVAL;
>
> So we could be reading beyond the end of the skb. If we got really
> unlucky it could lead to an Oops.
>
> regards,
> dan carpenter
>
>
Hello Dan,
The function rx_helper() in bh.c already do some sanity checks received data:
60 WARN(read_len < 4, "corrupted read");
[...]
92 } else {
93 computed_len = round_up(hif->len, 2);
94 }
95 if (computed_len != read_len) {
96 dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
97 computed_len, read_len);
98 print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET, 16, 1,
99 hif, read_len, true);
100 goto err;
101 }
However, I can improve this code:
- "4" should be replaced by "sizeof(struct hif_msg)" for readability
- hif->len is tested through computed_len, but I am not sure to be able
to prove that it covers all cases
- rx_helper() should recover the error if read_len < 4
I add that on my TODO list.
--
Jérôme Pouiller
next prev parent reply other threads:[~2020-04-02 14:44 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-01 11:03 [PATCH 00/32] staging: wfx: rework the Tx queue Jerome Pouiller
2020-04-01 11:03 ` [PATCH 01/32] staging: wfx: add sanity checks to hif_join() Jerome Pouiller
2020-04-02 12:42 ` Dan Carpenter
2020-04-02 16:14 ` Jérôme Pouiller
2020-04-01 11:03 ` [PATCH 02/32] staging: wfx: do not stop mac80211 queueing during tx_policy upload Jerome Pouiller
2020-04-01 11:03 ` [PATCH 03/32] staging: wfx: take advantage of ieee80211_{stop/start}_queues Jerome Pouiller
2020-04-01 11:03 ` [PATCH 04/32] staging: wfx: remove "burst" mechanism Jerome Pouiller
2020-04-02 13:05 ` Dan Carpenter
2020-04-02 14:47 ` Jérôme Pouiller
2020-04-01 11:03 ` [PATCH 05/32] staging: wfx: uniformize queue_id retrieval Jerome Pouiller
2020-04-01 11:03 ` [PATCH 06/32] staging: wfx: drop useless queue_id field Jerome Pouiller
2020-04-01 11:03 ` [PATCH 07/32] staging: wfx: avoid useless wake_up Jerome Pouiller
2020-04-01 11:03 ` [PATCH 08/32] staging: wfx: simplify hif_handle_tx_data() Jerome Pouiller
2020-04-02 13:13 ` Dan Carpenter
2020-04-02 14:44 ` Jérôme Pouiller [this message]
2020-04-01 11:03 ` [PATCH 09/32] staging: wfx: simplify wfx_tx_queues_empty() Jerome Pouiller
2020-04-01 11:03 ` [PATCH 10/32] staging: wfx: drop unused argument in wfx_get_prio_queue() Jerome Pouiller
2020-04-01 11:03 ` [PATCH 11/32] staging: wfx: simplify wfx_tx_queue_mask_get() Jerome Pouiller
2020-04-01 11:03 ` [PATCH 12/32] staging: wfx: drop useless sta_asleep_mask Jerome Pouiller
2020-04-01 11:03 ` [PATCH 13/32] staging: wfx: drop argument tx_allowed_mask since it is constant now Jerome Pouiller
2020-04-01 11:03 ` [PATCH 14/32] staging: wfx: do not use link_map_cache to track CAB Jerome Pouiller
2020-04-01 11:03 ` [PATCH 15/32] staging: wfx: drop useless link_map_cache Jerome Pouiller
2020-04-01 11:03 ` [PATCH 16/32] staging: wfx: do not rely anymore on link_id to choose packet in queue Jerome Pouiller
2020-04-01 11:03 ` [PATCH 17/32] staging: wfx: drop unused link_id field Jerome Pouiller
2020-04-01 11:03 ` [PATCH 18/32] staging: wfx: drop unused raw_link_id field Jerome Pouiller
2020-04-01 11:03 ` [PATCH 19/32] staging: wfx: rename wfx_tx_get_raw_link_id() Jerome Pouiller
2020-04-01 11:03 ` [PATCH 20/32] staging: wfx: replace wfx_tx_queues_get_after_dtim() by wfx_tx_queues_has_cab() Jerome Pouiller
2020-04-01 11:03 ` [PATCH 21/32] staging: wfx: introduce a counter of pending frames Jerome Pouiller
2020-04-01 11:03 ` [PATCH 22/32] staging: wfx: change the way to choose frame to send Jerome Pouiller
2020-04-01 11:03 ` [PATCH 23/32] staging: wfx: drop now useless field edca_params Jerome Pouiller
2020-04-01 11:03 ` [PATCH 24/32] staging: wfx: drop struct wfx_queue_stats Jerome Pouiller
2020-04-01 11:03 ` [PATCH 25/32] staging: wfx: simplify usage of wfx_tx_queues_put() Jerome Pouiller
2020-04-01 11:03 ` [PATCH 26/32] staging: wfx: improve interface between data_tx.c and queue.c Jerome Pouiller
2020-04-01 11:04 ` [PATCH 27/32] staging: wfx: relocate wfx_skb_dtor() prior its callers Jerome Pouiller
2020-04-01 11:04 ` [PATCH 28/32] staging: wfx: repair wfx_flush() Jerome Pouiller
2020-04-01 11:04 ` [PATCH 29/32] staging: wfx: wfx_flush() did not ensure that frames are processed Jerome Pouiller
2020-04-01 11:04 ` [PATCH 30/32] staging: wfx: fix potential deadlock in wfx_tx_flush() Jerome Pouiller
2020-04-01 11:04 ` [PATCH 31/32] staging: wfx: fix case where AP stop with CAB traffic pending Jerome Pouiller
2020-04-01 11:04 ` [PATCH 32/32] staging: wfx: remove hack about tx_rate policies Jerome Pouiller
2020-04-03 8:03 ` [PATCH 00/32] staging: wfx: rework the Tx queue Dan Carpenter
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=2302785.6C7ODC2LYm@pc-42 \
--to=jerome.pouiller@silabs.com \
--cc=dan.carpenter@oracle.com \
--cc=davem@davemloft.net \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.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.