* [PATCH] net: dsa: fix negative loop bound error on for loop
@ 2019-03-30 21:42 Colin King
2019-03-31 9:55 ` Mukesh Ojha
0 siblings, 1 reply; 4+ messages in thread
From: Colin King @ 2019-03-30 21:42 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently the for-loop using an unsigned int for the loop counter
which is problematic when comparing it to the signed int count
This is an issue because if the signed int is negative then
the negative loop bound is implicitly cast to an unsigned int on
the comparison to loop counter i and will yield a very large value,
eventually causing an error when memmove/memcpy'ing outside the
allocated region pointed to by ndata.
Fix this by simply making the loop counter i a signed int;
Fixes: f2f2356685bc ("net: dsa: move master ethtool code")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/dsa/master.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/dsa/master.c b/net/dsa/master.c
index c58f33931be1..1b659647a303 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -87,8 +87,7 @@ static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
struct dsa_switch *ds = cpu_dp->ds;
int port = cpu_dp->index;
int len = ETH_GSTRING_LEN;
- int mcount = 0, count;
- unsigned int i;
+ int mcount = 0, count, i;
uint8_t pfx[4];
uint8_t *ndata;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: dsa: fix negative loop bound error on for loop
2019-03-30 21:42 [PATCH] net: dsa: fix negative loop bound error on for loop Colin King
@ 2019-03-31 9:55 ` Mukesh Ojha
2019-03-31 14:33 ` Andrew Lunn
0 siblings, 1 reply; 4+ messages in thread
From: Mukesh Ojha @ 2019-03-31 9:55 UTC (permalink / raw)
To: Colin King, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
On 3/31/2019 3:12 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the for-loop using an unsigned int for the loop counter
> which is problematic when comparing it to the signed int count
> This is an issue because if the signed int is negative then
> the negative loop bound is implicitly cast to an unsigned int on
> the comparison to loop counter i and will yield a very large value,
> eventually causing an error when memmove/memcpy'ing outside the
> allocated region pointed to by ndata.
>
> Fix this by simply making the loop counter i a signed int;
>
> Fixes: f2f2356685bc ("net: dsa: move master ethtool code")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/dsa/master.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/dsa/master.c b/net/dsa/master.c
> index c58f33931be1..1b659647a303 100644
> --- a/net/dsa/master.c
> +++ b/net/dsa/master.c
> @@ -87,8 +87,7 @@ static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
> struct dsa_switch *ds = cpu_dp->ds;
> int port = cpu_dp->index;
> int len = ETH_GSTRING_LEN;
> - int mcount = 0, count;
> - unsigned int i;
> + int mcount = 0, count, i;
This looks fine but why the return value checking for the negative will
not be good here ?
count = ds->ops->get_sset_count(ds, port, stringset);
Cheers,
Mukesh
> uint8_t pfx[4];
> uint8_t *ndata;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: dsa: fix negative loop bound error on for loop
2019-03-31 9:55 ` Mukesh Ojha
@ 2019-03-31 14:33 ` Andrew Lunn
2019-04-01 5:41 ` Mukesh Ojha
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Lunn @ 2019-03-31 14:33 UTC (permalink / raw)
To: Mukesh Ojha
Cc: Colin King, Vivien Didelot, Florian Fainelli, David S . Miller,
netdev, kernel-janitors, linux-kernel
On Sun, Mar 31, 2019 at 03:25:47PM +0530, Mukesh Ojha wrote:
>
> On 3/31/2019 3:12 AM, Colin King wrote:
> >From: Colin Ian King <colin.king@canonical.com>
> >
> >Currently the for-loop using an unsigned int for the loop counter
> >which is problematic when comparing it to the signed int count
> >This is an issue because if the signed int is negative then
> >the negative loop bound is implicitly cast to an unsigned int on
> >the comparison to loop counter i and will yield a very large value,
> >eventually causing an error when memmove/memcpy'ing outside the
> >allocated region pointed to by ndata.
> >
> >Fix this by simply making the loop counter i a signed int;
> >
> >Fixes: f2f2356685bc ("net: dsa: move master ethtool code")
> >Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >---
> > net/dsa/master.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> >diff --git a/net/dsa/master.c b/net/dsa/master.c
> >index c58f33931be1..1b659647a303 100644
> >--- a/net/dsa/master.c
> >+++ b/net/dsa/master.c
> >@@ -87,8 +87,7 @@ static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
> > struct dsa_switch *ds = cpu_dp->ds;
> > int port = cpu_dp->index;
> > int len = ETH_GSTRING_LEN;
> >- int mcount = 0, count;
> >- unsigned int i;
> >+ int mcount = 0, count, i;
>
> This looks fine but why the return value checking for the negative will not
> be good here ?
> count = ds->ops->get_sset_count(ds, port, stringset);
Hi Mukesh
Is there a return value check for negative?
Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: dsa: fix negative loop bound error on for loop
2019-03-31 14:33 ` Andrew Lunn
@ 2019-04-01 5:41 ` Mukesh Ojha
0 siblings, 0 replies; 4+ messages in thread
From: Mukesh Ojha @ 2019-04-01 5:41 UTC (permalink / raw)
To: Andrew Lunn
Cc: Colin King, Vivien Didelot, Florian Fainelli, David S . Miller,
netdev, kernel-janitors, linux-kernel
On 3/31/2019 8:03 PM, Andrew Lunn wrote:
> On Sun, Mar 31, 2019 at 03:25:47PM +0530, Mukesh Ojha wrote:
>> On 3/31/2019 3:12 AM, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> Currently the for-loop using an unsigned int for the loop counter
>>> which is problematic when comparing it to the signed int count
>>> This is an issue because if the signed int is negative then
>>> the negative loop bound is implicitly cast to an unsigned int on
>>> the comparison to loop counter i and will yield a very large value,
>>> eventually causing an error when memmove/memcpy'ing outside the
>>> allocated region pointed to by ndata.
>>>
>>> Fix this by simply making the loop counter i a signed int;
>>>
>>> Fixes: f2f2356685bc ("net: dsa: move master ethtool code")
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> ---
>>> net/dsa/master.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/net/dsa/master.c b/net/dsa/master.c
>>> index c58f33931be1..1b659647a303 100644
>>> --- a/net/dsa/master.c
>>> +++ b/net/dsa/master.c
>>> @@ -87,8 +87,7 @@ static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
>>> struct dsa_switch *ds = cpu_dp->ds;
>>> int port = cpu_dp->index;
>>> int len = ETH_GSTRING_LEN;
>>> - int mcount = 0, count;
>>> - unsigned int i;
>>> + int mcount = 0, count, i;
>> This looks fine but why the return value checking for the negative will not
>> be good here ?
>> count = ds->ops->get_sset_count(ds, port, stringset);
> Hi Mukesh
>
> Is there a return value check for negative?
Just checked looks like dsa_master_get_sset_count can never return -ve
value .
Colin,
do we really get this situation in any scenario ?
Thanks.
Mukesh
Mukesh
>
> Andrew
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-01 5:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-30 21:42 [PATCH] net: dsa: fix negative loop bound error on for loop Colin King
2019-03-31 9:55 ` Mukesh Ojha
2019-03-31 14:33 ` Andrew Lunn
2019-04-01 5:41 ` Mukesh Ojha
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).