2022-08-12 03:47:13

by Jiapeng Chong

Subject: [PATCH] bonding: return -ENOMEM on rlb_initialize() allocation failure

drivers/net/bonding/bond_alb.c:861 rlb_initialize() warn: returning -1 instead of -ENOMEM is sloppy.

Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=1896
Reported-by: Abaci Robot <[email protected]>
Signed-off-by: Jiapeng Chong <[email protected]>
---
drivers/net/bonding/bond_alb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 60cb9a0225aa..96cb4404b3c7 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -858,7 +858,7 @@ static int rlb_initialize(struct bonding *bond)

new_hashtbl = kmalloc(size, GFP_KERNEL);
if (!new_hashtbl)
- return -1;
+ return -ENOMEM;

spin_lock_bh(&bond->mode_lock);

--
2.20.1.7.g153144c

2022-08-12 04:11:48

by Jay Vosburgh

Subject: Re: [PATCH] bonding: return -ENOMEM on rlb_initialize() allocation failure

Jiapeng Chong <[email protected]> wrote:

>drivers/net/bonding/bond_alb.c:861 rlb_initialize() warn: returning -1 instead of -ENOMEM is sloppy.

I'll disagree; the return value is only ever tested for being
non-zero.

-J

>Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=1896
>Reported-by: Abaci Robot <[email protected]>
>Signed-off-by: Jiapeng Chong <[email protected]>
>---
> drivers/net/bonding/bond_alb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index 60cb9a0225aa..96cb4404b3c7 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -858,7 +858,7 @@ static int rlb_initialize(struct bonding *bond)
>
> new_hashtbl = kmalloc(size, GFP_KERNEL);
> if (!new_hashtbl)
>- return -1;
>+ return -ENOMEM;
>
> spin_lock_bh(&bond->mode_lock);
>
>--
>2.20.1.7.g153144c
>

---
-Jay Vosburgh, [email protected]

2022-08-12 07:23:28

by Christophe JAILLET

Subject: Re: [PATCH] bonding: return -ENOMEM on rlb_initialize() allocation failure

Le 12/08/2022 à 05:20, Jiapeng Chong a écrit :
> drivers/net/bonding/bond_alb.c:861 rlb_initialize() warn: returning -1 instead of -ENOMEM is sloppy.
>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=1896
> Reported-by: Abaci Robot <[email protected]>
> Signed-off-by: Jiapeng Chong <[email protected]>
> ---
> drivers/net/bonding/bond_alb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 60cb9a0225aa..96cb4404b3c7 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -858,7 +858,7 @@ static int rlb_initialize(struct bonding *bond)
>
> new_hashtbl = kmalloc(size, GFP_KERNEL);
> if (!new_hashtbl)
> - return -1;
> + return -ENOMEM;
>
> spin_lock_bh(&bond->mode_lock);
>

Hi,

Nit: if of any use, the only call chain leads to [1]:
bond_open()
--> bond_alb_initialize()
--> rlb_initialize()

So, the error in bond_open() could be changed to ret instead of a hard
coded -ENOMEM.

Just my 2c,


Other than that, for what it worth,
Reviewed-by: Christophe JAILLET <[email protected]>

CJ

[1]:
https://elixir.bootlin.com/linux/v5.19/source/drivers/net/bonding/bond_main.c#L4163