Linux-WPAN Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net-next] ieee802154: ca8210: fix possible u8 overflow in ca8210_rx_done
@ 2018-12-11  3:13 YueHaibing
  2018-12-11  6:01 ` David Miller
  2018-12-11  8:39 ` Stefan Schmidt
  0 siblings, 2 replies; 5+ messages in thread
From: YueHaibing @ 2018-12-11  3:13 UTC (permalink / raw)
  To: davem, h.morris, alex.aring, stefan
  Cc: linux-kernel, netdev, linux-wpan, YueHaibing

gcc warning this:

drivers/net/ieee802154/ca8210.c:730:10: warning:
 comparison is always false due to limited range of data type [-Wtype-limits]

'len' is u8 type, we get it from buf[1] adding 2, which can overflow.
This patch change the type of 'len' to unsigned int to avoid this,also fix
the gcc warning.

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ieee802154/ca8210.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 0ff5a40..b2ff903 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -721,7 +721,7 @@ static void ca8210_mlme_reset_worker(struct work_struct *work)
 static void ca8210_rx_done(struct cas_control *cas_ctl)
 {
 	u8 *buf;
-	u8 len;
+	unsigned int len;
 	struct work_priv_container *mlme_reset_wpc;
 	struct ca8210_priv *priv = cas_ctl->priv;
 
@@ -730,7 +730,7 @@ static void ca8210_rx_done(struct cas_control *cas_ctl)
 	if (len > CA8210_SPI_BUF_SIZE) {
 		dev_crit(
 			&priv->spi->dev,
-			"Received packet len (%d) erroneously long\n",
+			"Received packet len (%u) erroneously long\n",
 			len
 		);
 		goto finish;
-- 
2.7.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] ieee802154: ca8210: fix possible u8 overflow in ca8210_rx_done
  2018-12-11  3:13 [PATCH net-next] ieee802154: ca8210: fix possible u8 overflow in ca8210_rx_done YueHaibing
@ 2018-12-11  6:01 ` David Miller
  2018-12-11  8:26   ` Stefan Schmidt
  2018-12-11  8:39 ` Stefan Schmidt
  1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2018-12-11  6:01 UTC (permalink / raw)
  To: yuehaibing; +Cc: h.morris, alex.aring, stefan, linux-kernel, netdev, linux-wpan

From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 11 Dec 2018 11:13:39 +0800

> gcc warning this:
> 
> drivers/net/ieee802154/ca8210.c:730:10: warning:
>  comparison is always false due to limited range of data type [-Wtype-limits]
> 
> 'len' is u8 type, we get it from buf[1] adding 2, which can overflow.
> This patch change the type of 'len' to unsigned int to avoid this,also fix
> the gcc warning.
> 
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

WPAN maintainers, I am assuming that as maintainers you will be
picking this up and sending it to me.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] ieee802154: ca8210: fix possible u8 overflow in ca8210_rx_done
  2018-12-11  6:01 ` David Miller
@ 2018-12-11  8:26   ` Stefan Schmidt
  2018-12-11 15:23     ` Alexander Aring
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Schmidt @ 2018-12-11  8:26 UTC (permalink / raw)
  To: David Miller, yuehaibing
  Cc: h.morris, alex.aring, linux-kernel, netdev, linux-wpan

Hello Dave.

On 11.12.18 07:01, David Miller wrote:
> From: YueHaibing <yuehaibing@huawei.com>
> Date: Tue, 11 Dec 2018 11:13:39 +0800
> 
>> gcc warning this:
>>
>> drivers/net/ieee802154/ca8210.c:730:10: warning:
>>  comparison is always false due to limited range of data type [-Wtype-limits]
>>
>> 'len' is u8 type, we get it from buf[1] adding 2, which can overflow.
>> This patch change the type of 'len' to unsigned int to avoid this,also fix
>> the gcc warning.
>>
>> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> 
> WPAN maintainers, I am assuming that as maintainers you will be
> picking this up and sending it to me.

That's correct. On driver patches I always wait 2 days or so to give the
driver maintainer a chance to reply before I go ahead and apply it.

I will take this one directly now and do some smoke testing. It will
come together with another fix as pull request to you.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] ieee802154: ca8210: fix possible u8 overflow in ca8210_rx_done
  2018-12-11  3:13 [PATCH net-next] ieee802154: ca8210: fix possible u8 overflow in ca8210_rx_done YueHaibing
  2018-12-11  6:01 ` David Miller
@ 2018-12-11  8:39 ` Stefan Schmidt
  1 sibling, 0 replies; 5+ messages in thread
From: Stefan Schmidt @ 2018-12-11  8:39 UTC (permalink / raw)
  To: YueHaibing, davem, h.morris, alex.aring; +Cc: linux-kernel, netdev, linux-wpan

Hello.

On 11.12.18 04:13, YueHaibing wrote:
> gcc warning this:
> 
> drivers/net/ieee802154/ca8210.c:730:10: warning:
>  comparison is always false due to limited range of data type [-Wtype-limits]
> 
> 'len' is u8 type, we get it from buf[1] adding 2, which can overflow.
> This patch change the type of 'len' to unsigned int to avoid this,also fix
> the gcc warning.
> 
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/net/ieee802154/ca8210.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index 0ff5a40..b2ff903 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -721,7 +721,7 @@ static void ca8210_mlme_reset_worker(struct work_struct *work)
>  static void ca8210_rx_done(struct cas_control *cas_ctl)
>  {
>  	u8 *buf;
> -	u8 len;
> +	unsigned int len;
>  	struct work_priv_container *mlme_reset_wpc;
>  	struct ca8210_priv *priv = cas_ctl->priv;
>  
> @@ -730,7 +730,7 @@ static void ca8210_rx_done(struct cas_control *cas_ctl)
>  	if (len > CA8210_SPI_BUF_SIZE) {
>  		dev_crit(
>  			&priv->spi->dev,
> -			"Received packet len (%d) erroneously long\n",
> +			"Received packet len (%u) erroneously long\n",
>  			len
>  		);
>  		goto finish;
> 


This patch has been applied to the wpan tree and will be
part of the next pull request to net. Thanks!

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] ieee802154: ca8210: fix possible u8 overflow in ca8210_rx_done
  2018-12-11  8:26   ` Stefan Schmidt
@ 2018-12-11 15:23     ` Alexander Aring
  0 siblings, 0 replies; 5+ messages in thread
From: Alexander Aring @ 2018-12-11 15:23 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: David Miller, yuehaibing, h.morris, alex.aring, linux-kernel,
	netdev, linux-wpan

Hi Stefan,

On Tue, Dec 11, 2018 at 09:26:37AM +0100, Stefan Schmidt wrote:
> Hello Dave.
> 
> On 11.12.18 07:01, David Miller wrote:
> > From: YueHaibing <yuehaibing@huawei.com>
> > Date: Tue, 11 Dec 2018 11:13:39 +0800
> > 
> >> gcc warning this:
> >>
> >> drivers/net/ieee802154/ca8210.c:730:10: warning:
> >>  comparison is always false due to limited range of data type [-Wtype-limits]
> >>
> >> 'len' is u8 type, we get it from buf[1] adding 2, which can overflow.
> >> This patch change the type of 'len' to unsigned int to avoid this,also fix
> >> the gcc warning.
> >>
> >> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> >> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > 
> > WPAN maintainers, I am assuming that as maintainers you will be
> > picking this up and sending it to me.
> 
> That's correct. On driver patches I always wait 2 days or so to give the
> driver maintainer a chance to reply before I go ahead and apply it.
> 
> I will take this one directly now and do some smoke testing. It will
> come together with another fix as pull request to you.
>

If this "len" is related to the frame (error message says packet) length when
"rx_done()" what the function name tells me, I think what the drivers
is looking for is:

ieee802154_is_valid_psdu_len() plus maybe before calculation of the length
depends what else the transceiver put as payload.

side note:

All drivers need to check if the length is valid as this is transmitted
in the PHY header and even not portected by CRC which is only for the
MAC payload. I had some ugly bufferoverflows expierence with some
at86rf2xx while my microwave was on.

- Alex

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-11  3:13 [PATCH net-next] ieee802154: ca8210: fix possible u8 overflow in ca8210_rx_done YueHaibing
2018-12-11  6:01 ` David Miller
2018-12-11  8:26   ` Stefan Schmidt
2018-12-11 15:23     ` Alexander Aring
2018-12-11  8:39 ` Stefan Schmidt

Linux-WPAN Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wpan/0 linux-wpan/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wpan linux-wpan/ https://lore.kernel.org/linux-wpan \
		linux-wpan@vger.kernel.org
	public-inbox-index linux-wpan

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wpan


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git