| 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
| ||
|
Message-ID: <20201008131320.GA1042@kadam>
Date: Thu, 8 Oct 2020 16:13:20 +0300
From: Dan Carpenter <dan.carpenter@...cle.com>
To: Jerome Pouiller <Jerome.Pouiller@...abs.com>
Cc: devel@...verdev.osuosl.org, linux-wireless@...r.kernel.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"David S . Miller" <davem@...emloft.net>,
Kalle Valo <kvalo@...eaurora.org>
Subject: Re: [PATCH 0/7] wfx: move out from the staging area
There are some static checker warnings to look at from linux-next from
Tuesday.
drivers/staging/wfx/hif_tx.c:319 hif_join() error: we previously assumed 'channel' could be null (see line 315)
drivers/staging/wfx/main.c:228 wfx_send_pdata_pds() warn: potential NULL parameter dereference 'tmp_buf'
drivers/staging/wfx/hif_rx.c:177 hif_scan_complete_indication() warn: potential NULL parameter dereference 'wvif'
drivers/staging/wfx/data_tx.c:576 wfx_flush() warn: potential NULL parameter dereference 'wvif'
drivers/staging/wfx/bus_spi.c:228 wfx_spi_probe() warn: 'bus->core' could be an error pointer
drivers/staging/wfx/bus_sdio.c:221 wfx_sdio_probe() warn: 'bus->core' could be an error pointer
drivers/staging/wfx/hif_rx.c:26 hif_generic_confirm() warn: negative user subtract: 0-u16max - 4
drivers/staging/wfx/hif_rx.c:98 hif_wakeup_indication() warn: 'gpiod_get_value(wdev->pdata.gpio_wakeup)' returns positive and negative
drivers/staging/wfx/bh.c:24 device_wakeup() warn: 'gpiod_get_value_cansleep(wdev->pdata.gpio_wakeup)' returns positive and negative
drivers/staging/wfx/hif_rx.c:235 hif_generic_indication() warn: format string contains non-ascii character '\xc2'
drivers/staging/wfx/hif_rx.c:235 hif_generic_indication() warn: format string contains non-ascii character '\xb0'
drivers/staging/wfx/data_tx.c:37 wfx_get_hw_rate() warn: constraint '(struct ieee80211_supported_band)->bitrates' overflow 'band->bitrates' 0 <= abs_rl '0-127' user_rl '' required = '(struct ieee80211_supported_band)->n_bitrates'
Some of these are unpublished checks that I haven't published because
they are too crap. The rest of the email is just long explanations.
Skip if not required.
regards,
dan carpenter
#1
drivers/staging/wfx/hif_tx.c:319 hif_join() error: we previously assumed 'channel' could be null (see line 315)
311 if (!hif)
312 return -ENOMEM;
313 body->infrastructure_bss_mode = !conf->ibss_joined;
314 body->short_preamble = conf->use_short_preamble;
315 if (channel && channel->flags & IEEE80211_CHAN_NO_IR)
^^^^^^^
Check for NULL.
316 body->probe_for_join = 0;
317 else
318 body->probe_for_join = 1;
319 body->channel_number = channel->hw_value;
^^^^^^^^^^^^^^^^^
Unchecked dereference.
320 body->beacon_interval = cpu_to_le32(conf->beacon_int);
321 body->basic_rate_set =
#2
drivers/staging/wfx/main.c:228 wfx_send_pdata_pds() warn: potential NULL parameter dereference 'tmp_buf'
227 tmp_buf = kmemdup(pds->data, pds->size, GFP_KERNEL);
No check for allocation failure.
228 ret = wfx_send_pds(wdev, tmp_buf, pds->size);
229 kfree(tmp_buf);
#3
drivers/staging/wfx/hif_rx.c:177 hif_scan_complete_indication() warn: potential NULL parameter dereference 'wvif'
170 static int hif_scan_complete_indication(struct wfx_dev *wdev,
171 const struct hif_msg *hif,
172 const void *buf)
173 {
174 struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
^^^^^^^^^^^^^^^^^^^
Smatch thinks wdev_to_wvif() can return NULL.
175
176 WARN_ON(!wvif);
177 wfx_scan_complete(wvif);
178
179 return 0;
180 }
#4
drivers/staging/wfx/data_tx.c:576 wfx_flush() warn: potential NULL parameter dereference 'wvif'
572 while ((skb = skb_dequeue(&dropped)) != NULL) {
573 hif = (struct hif_msg *)skb->data;
574 wvif = wdev_to_wvif(wdev, hif->interface);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Same.
575 ieee80211_tx_info_clear_status(IEEE80211_SKB_CB(skb));
576 wfx_skb_dtor(wvif, skb);
577 }
578 }
#5 and #6
drivers/staging/wfx/bus_spi.c:228 wfx_spi_probe() warn: 'bus->core' could be an error pointer
drivers/staging/wfx/bus_sdio.c:221 wfx_sdio_probe() warn: 'bus->core' could be an error pointer
The wfx_init_common() function should return NULL instead of error
pointer if devm_gpiod_get_optional() fails.
#7
drivers/staging/wfx/hif_rx.c:26 hif_generic_confirm() warn: negative user subtract: 0-u16max - 4
20 static int hif_generic_confirm(struct wfx_dev *wdev,
21 const struct hif_msg *hif, const void *buf)
22 {
23 // All confirm messages start with status
24 int status = le32_to_cpup((__le32 *)buf);
25 int cmd = hif->id;
26 int len = le16_to_cpu(hif->len) - 4; // drop header
Can "len" get set to negative 4?
27
28 WARN(!mutex_is_locked(&wdev->hif_cmd.lock), "data locking error");
#8 and #9
drivers/staging/wfx/hif_rx.c:98 hif_wakeup_indication() warn: 'gpiod_get_value(wdev->pdata.gpio_wakeup)' returns positive and negative
drivers/staging/wfx/bh.c:24 device_wakeup() warn: 'gpiod_get_value_cansleep(wdev->pdata.gpio_wakeup)' returns positive and negative
94 static int hif_wakeup_indication(struct wfx_dev *wdev,
95 const struct hif_msg *hif, const void *buf)
96 {
97 if (!wdev->pdata.gpio_wakeup
98 || !gpiod_get_value(wdev->pdata.gpio_wakeup)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Negative error codes from gpiod_get_value() should be treated as error.
99 dev_warn(wdev->dev, "unexpected wake-up indication\n");
100 return -EIO;
101 }
102 return 0;
103 }
#10 and #11
drivers/staging/wfx/hif_rx.c:235 hif_generic_indication() warn: format string contains non-ascii character '\xc2'
drivers/staging/wfx/hif_rx.c:235 hif_generic_indication() warn: format string contains non-ascii character '\xb0'
234 if (!wfx_api_older_than(wdev, 1, 4))
235 dev_info(wdev->dev, "Rx test ongoing. Temperature: %d°C\n",
^
Can we output non-ascii to dmesg? (I didn't add this Smatch check so I
don't really know the answer).
236 body->data.rx_stats.current_temp);
#12
drivers/staging/wfx/data_tx.c:37 wfx_get_hw_rate() warn: constraint '(struct ieee80211_supported_band)->bitrates' overflow 'band->bitrates' 0 <= abs_rl '0-127' user_rl '' required = '(struct ieee80211_supported_band)->n_bitrates'
20 static int wfx_get_hw_rate(struct wfx_dev *wdev,
21 const struct ieee80211_tx_rate *rate)
22 {
23 struct ieee80211_supported_band *band;
24
25 if (rate->idx < 0)
26 return -1;
27 if (rate->flags & IEEE80211_TX_RC_MCS) {
28 if (rate->idx > 7) {
29 WARN(1, "wrong rate->idx value: %d", rate->idx);
30 return -1;
31 }
32 return rate->idx + 14;
33 }
34 // WFx only support 2GHz, else band information should be retrieved
35 // from ieee80211_tx_info
36 band = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
37 return band->bitrates[rate->idx].hw_value;
This code assumes that "rate->idx" can be all sort of invalid values
including negatives but it doesn't cap it against:
if (rate->idx >= band->n_bitrates)
return -1;
38 }
If you you read all the way down to the second end of the email then you
are a true hero. regards again,
dan carpenter
Powered by blists - more mailing lists