* [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx()
@ 2023-03-06 19:18 Harshit Mogalapalli
2023-03-07 0:06 ` Alexander Aring
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Harshit Mogalapalli @ 2023-03-06 19:18 UTC (permalink / raw)
Cc: error27, Harshit Mogalapalli, Alexander Aring, Stefan Schmidt,
Miquel Raynal, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Marcel Holtmann, Harry Morris, linux-wpan, netdev,
linux-kernel
mac_len is of type unsigned, which can never be less than zero.
mac_len = ieee802154_hdr_peek_addrs(skb, &header);
if (mac_len < 0)
return mac_len;
Change this to type int as ieee802154_hdr_peek_addrs() can return negative
integers, this is found by static analysis with smatch.
Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
Only compile tested.
---
drivers/net/ieee802154/ca8210.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 0b0c6c0764fe..d0b5129439ed 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -1902,10 +1902,9 @@ static int ca8210_skb_tx(
struct ca8210_priv *priv
)
{
- int status;
struct ieee802154_hdr header = { };
struct secspec secspec;
- unsigned int mac_len;
+ int mac_len, status;
dev_dbg(&priv->spi->dev, "%s called\n", __func__);
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx()
2023-03-06 19:18 [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx() Harshit Mogalapalli
@ 2023-03-07 0:06 ` Alexander Aring
2023-03-07 8:42 ` Simon Horman
2023-03-16 16:31 ` Stefan Schmidt
2 siblings, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2023-03-07 0:06 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: error27, Alexander Aring, Stefan Schmidt, Miquel Raynal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Marcel Holtmann, Harry Morris, linux-wpan, netdev, linux-kernel
Hi,
On Mon, Mar 6, 2023 at 2:20 PM Harshit Mogalapalli
<harshit.m.mogalapalli@oracle.com> wrote:
>
> mac_len is of type unsigned, which can never be less than zero.
>
> mac_len = ieee802154_hdr_peek_addrs(skb, &header);
> if (mac_len < 0)
> return mac_len;
>
> Change this to type int as ieee802154_hdr_peek_addrs() can return negative
> integers, this is found by static analysis with smatch.
>
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Acked-by: Alexander Aring <aahringo@redhat.com>
sorry, I didn't see that... Thanks for sending this patch.
- Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx()
2023-03-06 19:18 [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx() Harshit Mogalapalli
2023-03-07 0:06 ` Alexander Aring
@ 2023-03-07 8:42 ` Simon Horman
2023-03-07 23:29 ` Alexander Aring
2023-03-16 16:33 ` Stefan Schmidt
2023-03-16 16:31 ` Stefan Schmidt
2 siblings, 2 replies; 7+ messages in thread
From: Simon Horman @ 2023-03-07 8:42 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: Dan Carpenter, Alexander Aring, Stefan Schmidt, Miquel Raynal,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Marcel Holtmann, Harry Morris, linux-wpan, netdev, linux-kernel
On Mon, Mar 06, 2023 at 11:18:24AM -0800, Harshit Mogalapalli wrote:
> mac_len is of type unsigned, which can never be less than zero.
>
> mac_len = ieee802154_hdr_peek_addrs(skb, &header);
> if (mac_len < 0)
> return mac_len;
>
> Change this to type int as ieee802154_hdr_peek_addrs() can return negative
> integers, this is found by static analysis with smatch.
>
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
I discussed this briefly with Harshit offline.
The commit referenced above tag does add the call to
ieee802154_hdr_peek_addrs(), an there is a sign miss match between
the return value and the variable.
The code to check the mac_len was added more recently, by the following
commit. However the fixes tag is probably fine as-is, because it's fixing
error handling of a call made in that commit.
6c993779ea1d ("ca8210: fix mac_len negative array access")
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx()
2023-03-07 8:42 ` Simon Horman
@ 2023-03-07 23:29 ` Alexander Aring
2023-03-16 16:33 ` Stefan Schmidt
1 sibling, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2023-03-07 23:29 UTC (permalink / raw)
To: Simon Horman
Cc: Harshit Mogalapalli, Dan Carpenter, Alexander Aring,
Stefan Schmidt, Miquel Raynal, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Marcel Holtmann, Harry Morris,
linux-wpan, netdev, linux-kernel
Hi,
On Tue, Mar 7, 2023 at 3:44 AM Simon Horman <simon.horman@corigine.com> wrote:
>
> On Mon, Mar 06, 2023 at 11:18:24AM -0800, Harshit Mogalapalli wrote:
> > mac_len is of type unsigned, which can never be less than zero.
> >
> > mac_len = ieee802154_hdr_peek_addrs(skb, &header);
> > if (mac_len < 0)
> > return mac_len;
> >
> > Change this to type int as ieee802154_hdr_peek_addrs() can return negative
> > integers, this is found by static analysis with smatch.
> >
> > Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>
> I discussed this briefly with Harshit offline.
>
> The commit referenced above tag does add the call to
> ieee802154_hdr_peek_addrs(), an there is a sign miss match between
> the return value and the variable.
>
> The code to check the mac_len was added more recently, by the following
> commit. However the fixes tag is probably fine as-is, because it's fixing
> error handling of a call made in that commit.
>
> 6c993779ea1d ("ca8210: fix mac_len negative array access")
>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
>
sure, thanks for catching this.
- Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx()
2023-03-06 19:18 [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx() Harshit Mogalapalli
2023-03-07 0:06 ` Alexander Aring
2023-03-07 8:42 ` Simon Horman
@ 2023-03-16 16:31 ` Stefan Schmidt
2023-03-16 19:02 ` Harshit Mogalapalli
2 siblings, 1 reply; 7+ messages in thread
From: Stefan Schmidt @ 2023-03-16 16:31 UTC (permalink / raw)
To: Harshit Mogalapalli
Cc: error27, Alexander Aring, Miquel Raynal, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marcel Holtmann,
Harry Morris, linux-wpan, netdev, linux-kernel
Hello Harshit.
On 06.03.23 20:18, Harshit Mogalapalli wrote:
> mac_len is of type unsigned, which can never be less than zero.
>
> mac_len = ieee802154_hdr_peek_addrs(skb, &header);
> if (mac_len < 0)
> return mac_len;
>
> Change this to type int as ieee802154_hdr_peek_addrs() can return negative
> integers, this is found by static analysis with smatch.
>
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
> Only compile tested.
> ---
> drivers/net/ieee802154/ca8210.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index 0b0c6c0764fe..d0b5129439ed 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -1902,10 +1902,9 @@ static int ca8210_skb_tx(
> struct ca8210_priv *priv
> )
> {
> - int status;
> struct ieee802154_hdr header = { };
> struct secspec secspec;
> - unsigned int mac_len;
> + int mac_len, status;
>
> dev_dbg(&priv->spi->dev, "%s called\n", __func__);
>
This patch has been applied to the wpan tree and will be
part of the next pull request to net. Thanks!
I took the liberty and changed the fixes tag to the change that
introduced the resaon for the mismatch recently. As suggested by Simon.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx()
2023-03-07 8:42 ` Simon Horman
2023-03-07 23:29 ` Alexander Aring
@ 2023-03-16 16:33 ` Stefan Schmidt
1 sibling, 0 replies; 7+ messages in thread
From: Stefan Schmidt @ 2023-03-16 16:33 UTC (permalink / raw)
To: Simon Horman, Harshit Mogalapalli
Cc: Dan Carpenter, Alexander Aring, Miquel Raynal, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marcel Holtmann,
Harry Morris, linux-wpan, netdev, linux-kernel
Hello Simon.
On 07.03.23 09:42, Simon Horman wrote:
> On Mon, Mar 06, 2023 at 11:18:24AM -0800, Harshit Mogalapalli wrote:
>> mac_len is of type unsigned, which can never be less than zero.
>>
>> mac_len = ieee802154_hdr_peek_addrs(skb, &header);
>> if (mac_len < 0)
>> return mac_len;
>>
>> Change this to type int as ieee802154_hdr_peek_addrs() can return negative
>> integers, this is found by static analysis with smatch.
>>
>> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>
> I discussed this briefly with Harshit offline.
>
> The commit referenced above tag does add the call to
> ieee802154_hdr_peek_addrs(), an there is a sign miss match between
> the return value and the variable.
>
> The code to check the mac_len was added more recently, by the following
> commit. However the fixes tag is probably fine as-is, because it's fixing
> error handling of a call made in that commit.
>
> 6c993779ea1d ("ca8210: fix mac_len negative array access")
>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
I agree that the commit above is the better Fixes tag as it makes clear
it only comes after this change. I amended the commit message
accordingly when applying this to wpan.
regards
Stefan Schmidt
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx()
2023-03-16 16:31 ` Stefan Schmidt
@ 2023-03-16 19:02 ` Harshit Mogalapalli
0 siblings, 0 replies; 7+ messages in thread
From: Harshit Mogalapalli @ 2023-03-16 19:02 UTC (permalink / raw)
To: Stefan Schmidt
Cc: error27, Alexander Aring, Miquel Raynal, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marcel Holtmann,
Harry Morris, linux-wpan, netdev, linux-kernel
Hi Stefan,
On 16/03/23 10:01 pm, Stefan Schmidt wrote:
> Hello Harshit.
>
> On 06.03.23 20:18, Harshit Mogalapalli wrote:
>> mac_len is of type unsigned, which can never be less than zero.
>>
>> mac_len = ieee802154_hdr_peek_addrs(skb, &header);
>> if (mac_len < 0)
>> return mac_len;
>>
>> Change this to type int as ieee802154_hdr_peek_addrs() can return
>> negative
>> integers, this is found by static analysis with smatch.
>>
>> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device
>> driver")
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> ---
>> Only compile tested.
>> ---
>> drivers/net/ieee802154/ca8210.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ieee802154/ca8210.c
>> b/drivers/net/ieee802154/ca8210.c
>> index 0b0c6c0764fe..d0b5129439ed 100644
>> --- a/drivers/net/ieee802154/ca8210.c
>> +++ b/drivers/net/ieee802154/ca8210.c
>> @@ -1902,10 +1902,9 @@ static int ca8210_skb_tx(
>> struct ca8210_priv *priv
>> )
>> {
>> - int status;
>> struct ieee802154_hdr header = { };
>> struct secspec secspec;
>> - unsigned int mac_len;
>> + int mac_len, status;
>> dev_dbg(&priv->spi->dev, "%s called\n", __func__);
>
> This patch has been applied to the wpan tree and will be
> part of the next pull request to net. Thanks!
>
> I took the liberty and changed the fixes tag to the change that
> introduced the resaon for the mismatch recently. As suggested by Simon.
>
Thanks for doing this, I wasn't very clear whether to change the Fixes
tag and send a v2.
Regards,
Harshit
> regards
> Stefan Schmidt
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-03-16 19:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-06 19:18 [PATCH next] ca8210: Fix unsigned mac_len comparison with zero in ca8210_skb_tx() Harshit Mogalapalli
2023-03-07 0:06 ` Alexander Aring
2023-03-07 8:42 ` Simon Horman
2023-03-07 23:29 ` Alexander Aring
2023-03-16 16:33 ` Stefan Schmidt
2023-03-16 16:31 ` Stefan Schmidt
2023-03-16 19:02 ` Harshit Mogalapalli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).