* [net v1] devlink: fix xa_alloc_cyclic error handling
@ 2025-02-14 13:24 Michal Swiatkowski
2025-02-14 13:44 ` Andrew Lunn
0 siblings, 1 reply; 13+ messages in thread
From: Michal Swiatkowski @ 2025-02-14 13:24 UTC (permalink / raw)
To: netdev
Cc: jiri, davem, edumazet, kuba, pabeni, horms, pierre,
Michal Swiatkowski
Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
from xa_alloc_cyclic() in scheduler code [1]. The same is done in
devlink_rel_alloc().
In case of returning 1 from xa_alloc_cyclic() (wrapping) ERR_PTR(1) will
be returned, which will cause IS_ERR() to be false. Which can lead to
dereference not allocated pointer (rel).
Fix it by checking if err is lower than zero.
This wasn't found in real usecase, only noticed. Credit to Pierre.
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
net/devlink/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/devlink/core.c b/net/devlink/core.c
index f49cd83f1955..7203c39532fc 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -117,7 +117,7 @@ static struct devlink_rel *devlink_rel_alloc(void)
err = xa_alloc_cyclic(&devlink_rels, &rel->index, rel,
xa_limit_32b, &next, GFP_KERNEL);
- if (err) {
+ if (err < 0) {
kfree(rel);
return ERR_PTR(err);
}
--
2.42.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-14 13:24 [net v1] devlink: fix xa_alloc_cyclic error handling Michal Swiatkowski
@ 2025-02-14 13:44 ` Andrew Lunn
2025-02-14 13:58 ` Michal Swiatkowski
2025-02-16 15:06 ` Dan Carpenter
0 siblings, 2 replies; 13+ messages in thread
From: Andrew Lunn @ 2025-02-14 13:44 UTC (permalink / raw)
To: Michal Swiatkowski
Cc: netdev, jiri, davem, edumazet, kuba, pabeni, horms, pierre,
Dan Carpenter
On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> devlink_rel_alloc().
If the same bug exists twice it might exist more times. Did you find
this instance by searching the whole tree? Or just networking?
This is also something which would be good to have the static
analysers check for. I wounder if smatch can check this?
Andrew
>
> In case of returning 1 from xa_alloc_cyclic() (wrapping) ERR_PTR(1) will
> be returned, which will cause IS_ERR() to be false. Which can lead to
> dereference not allocated pointer (rel).
>
> Fix it by checking if err is lower than zero.
>
> This wasn't found in real usecase, only noticed. Credit to Pierre.
>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
> net/devlink/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/devlink/core.c b/net/devlink/core.c
> index f49cd83f1955..7203c39532fc 100644
> --- a/net/devlink/core.c
> +++ b/net/devlink/core.c
> @@ -117,7 +117,7 @@ static struct devlink_rel *devlink_rel_alloc(void)
>
> err = xa_alloc_cyclic(&devlink_rels, &rel->index, rel,
> xa_limit_32b, &next, GFP_KERNEL);
> - if (err) {
> + if (err < 0) {
> kfree(rel);
> return ERR_PTR(err);
> }
> --
> 2.42.0
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-14 13:44 ` Andrew Lunn
@ 2025-02-14 13:58 ` Michal Swiatkowski
2025-02-14 14:14 ` Andrew Lunn
2025-02-18 11:56 ` Paolo Abeni
2025-02-16 15:06 ` Dan Carpenter
1 sibling, 2 replies; 13+ messages in thread
From: Michal Swiatkowski @ 2025-02-14 13:58 UTC (permalink / raw)
To: Andrew Lunn
Cc: Michal Swiatkowski, netdev, jiri, davem, edumazet, kuba, pabeni,
horms, pierre, Dan Carpenter
On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
> On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> > Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> > from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> > devlink_rel_alloc().
>
> If the same bug exists twice it might exist more times. Did you find
> this instance by searching the whole tree? Or just networking?
>
> This is also something which would be good to have the static
> analysers check for. I wounder if smatch can check this?
>
> Andrew
>
You are right, I checked only net folder and there are two usage like
that in drivers. I will send v2 with wider fixing, thanks.
It can be not so easy to check. What if someone want to treat wrapping
as an error (don't know if it is valid)? If one of the caller is
checking err < 0 it will be fine.
Thanks,
Michal
> >
> > In case of returning 1 from xa_alloc_cyclic() (wrapping) ERR_PTR(1) will
> > be returned, which will cause IS_ERR() to be false. Which can lead to
> > dereference not allocated pointer (rel).
> >
> > Fix it by checking if err is lower than zero.
> >
> > This wasn't found in real usecase, only noticed. Credit to Pierre.
> >
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > ---
> > net/devlink/core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/devlink/core.c b/net/devlink/core.c
> > index f49cd83f1955..7203c39532fc 100644
> > --- a/net/devlink/core.c
> > +++ b/net/devlink/core.c
> > @@ -117,7 +117,7 @@ static struct devlink_rel *devlink_rel_alloc(void)
> >
> > err = xa_alloc_cyclic(&devlink_rels, &rel->index, rel,
> > xa_limit_32b, &next, GFP_KERNEL);
> > - if (err) {
> > + if (err < 0) {
> > kfree(rel);
> > return ERR_PTR(err);
> > }
> > --
> > 2.42.0
> >
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-14 13:58 ` Michal Swiatkowski
@ 2025-02-14 14:14 ` Andrew Lunn
2025-02-18 11:56 ` Paolo Abeni
1 sibling, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2025-02-14 14:14 UTC (permalink / raw)
To: Michal Swiatkowski
Cc: netdev, jiri, davem, edumazet, kuba, pabeni, horms, pierre,
Dan Carpenter
On Fri, Feb 14, 2025 at 02:58:41PM +0100, Michal Swiatkowski wrote:
> On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
> > On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> > > Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> > > from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> > > devlink_rel_alloc().
> >
> > If the same bug exists twice it might exist more times. Did you find
> > this instance by searching the whole tree? Or just networking?
> >
> > This is also something which would be good to have the static
> > analysers check for. I wounder if smatch can check this?
> >
> > Andrew
> >
>
> You are right, I checked only net folder and there are two usage like
> that in drivers. I will send v2 with wider fixing, thanks.
>
> It can be not so easy to check. What if someone want to treat wrapping
> as an error (don't know if it is valid)? If one of the caller is
> checking err < 0 it will be fine.
I put Dan in Cc:, lets see what he thinks.
There is at least one other functions i can think of which has similar
behaviour, < 0 on error, 0 or 1 are both different sorts of
success. If there are two, there are probably more. Having tooling to
find this sort of problem would be nice, even if it has a high false
positive rate and needs combining with manual inspection.
Andrew
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-14 13:44 ` Andrew Lunn
2025-02-14 13:58 ` Michal Swiatkowski
@ 2025-02-16 15:06 ` Dan Carpenter
2025-02-16 16:08 ` Andrew Lunn
2025-02-17 6:57 ` Dan Carpenter
1 sibling, 2 replies; 13+ messages in thread
From: Dan Carpenter @ 2025-02-16 15:06 UTC (permalink / raw)
To: Andrew Lunn
Cc: Michal Swiatkowski, netdev, jiri, davem, edumazet, kuba, pabeni,
horms, pierre, Dan Carpenter
On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
> On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> > Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> > from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> > devlink_rel_alloc().
>
> If the same bug exists twice it might exist more times. Did you find
> this instance by searching the whole tree? Or just networking?
>
> This is also something which would be good to have the static
> analysers check for. I wounder if smatch can check this?
That's a great idea, thanks! I'll try a couple experiments and see what
works tomorrow. I've add these lines to check_zero_to_err_ptr.c
183 max = rl_max(estate_rl(sm->state));
184 if (max.value > 0 && !sval_is_a_max(max))
185 sm_warning("passing non-max range '%s' to '%s'", sm->state->name, fn);
186
I'm hoping this one works. It complains about any positive returns
except for when the return is "some non-zero value".
194 if (estate_get_single_value(tmp->state, &sval) &&
195 (sval.value < -4096 || sval.value > 0)) {
196 sm_warning("passing invalid error code %lld to '%s'", sval.value, fn);
197 return;
198 }
This one might miss some bugs but it should catch most stuff and have few
false positives. Both of them work on this example.
net/devlink/core.c:122 devlink_rel_alloc() warn: passing non-max range '(-4095)-(-1),1' to 'ERR_PTR'
net/devlink/core.c:122 devlink_rel_alloc() warn: passing invalid error code 1 to 'ERR_PTR'
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-16 15:06 ` Dan Carpenter
@ 2025-02-16 16:08 ` Andrew Lunn
2025-02-17 7:46 ` Dan Carpenter
2025-02-17 6:57 ` Dan Carpenter
1 sibling, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2025-02-16 16:08 UTC (permalink / raw)
To: Dan Carpenter
Cc: Michal Swiatkowski, netdev, jiri, davem, edumazet, kuba, pabeni,
horms, pierre, Dan Carpenter
On Sun, Feb 16, 2025 at 06:06:48PM +0300, Dan Carpenter wrote:
> On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
> > On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> > > Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> > > from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> > > devlink_rel_alloc().
> >
> > If the same bug exists twice it might exist more times. Did you find
> > this instance by searching the whole tree? Or just networking?
> >
> > This is also something which would be good to have the static
> > analysers check for. I wounder if smatch can check this?
>
> That's a great idea, thanks! I'll try a couple experiments and see what
> works tomorrow. I've add these lines to check_zero_to_err_ptr.c
>
> 183 max = rl_max(estate_rl(sm->state));
> 184 if (max.value > 0 && !sval_is_a_max(max))
> 185 sm_warning("passing non-max range '%s' to '%s'", sm->state->name, fn);
> 186
>
> I'm hoping this one works. It complains about any positive returns
> except for when the return is "some non-zero value".
>
> 194 if (estate_get_single_value(tmp->state, &sval) &&
> 195 (sval.value < -4096 || sval.value > 0)) {
> 196 sm_warning("passing invalid error code %lld to '%s'", sval.value, fn);
> 197 return;
> 198 }
>
> This one might miss some bugs but it should catch most stuff and have few
> false positives. Both of them work on this example.
>
> net/devlink/core.c:122 devlink_rel_alloc() warn: passing non-max range '(-4095)-(-1),1' to 'ERR_PTR'
> net/devlink/core.c:122 devlink_rel_alloc() warn: passing invalid error code 1 to 'ERR_PTR'
Nice. In networking, ethernet PHYs, there are a few functions ending
in _changed() have the same behaviour:
* Returns negative errno, 0 if there was no change, and 1 in case of change
So there is the potential for the same issue with
mdiobus_modify_changed(), phy_modify_changed(),
phy_modify_mmd_changed(), phy_modify_paged_changed(). Hope this helps
with testing.
Andrew
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-16 15:06 ` Dan Carpenter
2025-02-16 16:08 ` Andrew Lunn
@ 2025-02-17 6:57 ` Dan Carpenter
1 sibling, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2025-02-17 6:57 UTC (permalink / raw)
To: Andrew Lunn
Cc: Michal Swiatkowski, netdev, jiri, davem, edumazet, kuba, pabeni,
horms, pierre, Dan Carpenter
Both versions find 97 warning but they're slightly different so I should
create some kind of combined check.
drivers/input/touchscreen/cyttsp_core.c:658 cyttsp_probe() warn: passing non-max range 's32min-(-1),1' to 'ERR_PTR'
drivers/gpu/drm/mediatek/mtk_dp.c:2736 mtk_dp_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/clk/clk-gpio.c:371 clk_gated_fixed_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/net/ethernet/socionext/netsec.c:1902 netsec_acpi_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/net/ethernet/socionext/netsec.c:1909 netsec_acpi_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c:2050 mcp251xfd_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/net/can/spi/hi311x.c:857 hi3110_can_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/net/can/dev/dev.c:496 can_get_termination() warn: passing non-max range 's32min-(-1),1' to 'ERR_PTR'
drivers/pwm/pwm-sl28cpld.c:228 sl28cpld_pwm_probe() warn: passing non-max range 's32min-(-1),1' to 'ERR_PTR'
drivers/leds/leds-is31fl319x.c:423 is31fl319x_parse_fw() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/leds/rgb/leds-mt6370-rgb.c:738 mt6370_assign_multicolor_info() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/leds/rgb/leds-mt6370-rgb.c:945 mt6370_leds_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/leds/rgb/leds-mt6370-rgb.c:952 mt6370_leds_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
The first return from acpi_data_prop_read() could be 1. I'll report
this.
drivers/gpu/drm/msm/msm_gem_submit.c:537 msm_parse_deps() warn: passing non-max range '(-4095)-(-1),22' to 'ERR_PTR'
Clear bug. Double negative -(-EINVAL). I'll send a patch.
drivers/gpu/drm/xe/xe_guc.c:1241 xe_guc_suspend() warn: passing non-max range '(-110),(-71),(-6),1-268435455' to 'ERR_PTR'
drivers/gpu/drm/i915/gt/uc/intel_guc.c:698 intel_guc_suspend() warn: passing non-max range '1-268435455' to 'ERR_PTR'
Smatch gets confused by the return FIELD_GET(GUC_HXG_RESPONSE_MSG_0_DATA0, header);
in xe_guc_mmio_send_recv().
drivers/spi/spi-pxa2xx-platform.c:101 pxa2xx_spi_init_pdata() warn: passing non-max range 's32min-(-1),1' to 'ERR_PTR'
drivers/spi/spi-cs42l43.c:396 cs42l43_spi_probe() warn: passing non-max range 's32min-(-23),(-21)-(-1),1' to 'dev_err_probe'
drivers/spi/spi-rockchip-sfc.c:654 rockchip_sfc_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/net/dsa/mv88e6xxx/pcs-6352.c:250 marvell_c22_pcs_link_up() warn: passing non-max range '(-110),(-95),(-16),1' to 'ERR_PTR'
False positive. marvell_c22_pcs_restore_page() is complicated.
drivers/net/phy/phy_device.c:1150 phy_connect() warn: passing non-max range 's32min-(-1),1' to 'ERR_PTR'
drivers/net/phy/phy_device.c:1651 phy_attach() warn: passing non-max range 's32min-(-1),1' to 'ERR_PTR'
drivers/net/ethernet/ethoc.c:719 ethoc_mdio_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c:178 hbg_phy_connect() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
xa_alloc_cyclic() returns 1 if the allocation succeeded after wrapping.
I'll report this.
drivers/net/ethernet/aquantia/atlantic/aq_ring.c:481 aq_xdp_run_prog() warn: passing non-max range '(-16),0,5-6' to 'ERR_PTR'
Bug. Mixing error codes from aq_hw_err_from_flags() and NETDEV_TX_BUSY.
drivers/media/i2c/ds90ub913.c:856 ub913_probe() warn: passing non-max range 's32min-(-1),1' to 'dev_err_probe'
This is a transient bug that went away after a database rebuild. I'll
debug the rest over time.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-16 16:08 ` Andrew Lunn
@ 2025-02-17 7:46 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2025-02-17 7:46 UTC (permalink / raw)
To: Andrew Lunn
Cc: Michal Swiatkowski, netdev, jiri, davem, edumazet, kuba, pabeni,
horms, pierre, Dan Carpenter
On Sun, Feb 16, 2025 at 05:08:23PM +0100, Andrew Lunn wrote:
> * Returns negative errno, 0 if there was no change, and 1 in case of change
>
> So there is the potential for the same issue with
> mdiobus_modify_changed(), phy_modify_changed(),
> phy_modify_mmd_changed(), phy_modify_paged_changed(). Hope this helps
> with testing.
Thanks, that's useful.
The first check would have caught all these, but the second one would
only have caught the first three. The combined check will catch them
all. :)
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-14 13:58 ` Michal Swiatkowski
2025-02-14 14:14 ` Andrew Lunn
@ 2025-02-18 11:56 ` Paolo Abeni
2025-03-10 11:42 ` Pierre Riteau
1 sibling, 1 reply; 13+ messages in thread
From: Paolo Abeni @ 2025-02-18 11:56 UTC (permalink / raw)
To: Michal Swiatkowski, Andrew Lunn
Cc: netdev, jiri, davem, edumazet, kuba, horms, pierre, Dan Carpenter
On 2/14/25 2:58 PM, Michal Swiatkowski wrote:
> On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
>> On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
>>> Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
>>> from xa_alloc_cyclic() in scheduler code [1]. The same is done in
>>> devlink_rel_alloc().
>>
>> If the same bug exists twice it might exist more times. Did you find
>> this instance by searching the whole tree? Or just networking?
>>
>> This is also something which would be good to have the static
>> analysers check for. I wounder if smatch can check this?
>>
>> Andrew
>>
>
> You are right, I checked only net folder and there are two usage like
> that in drivers. I will send v2 with wider fixing, thanks.
While at that, please add the suitable fixes tag(s).
Thanks,
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-02-18 11:56 ` Paolo Abeni
@ 2025-03-10 11:42 ` Pierre Riteau
2025-03-11 9:16 ` Michal Swiatkowski
0 siblings, 1 reply; 13+ messages in thread
From: Pierre Riteau @ 2025-03-10 11:42 UTC (permalink / raw)
To: Paolo Abeni
Cc: Michal Swiatkowski, Andrew Lunn, netdev, jiri, davem, edumazet,
kuba, horms, Dan Carpenter
On Tue, 18 Feb 2025 at 12:56, Paolo Abeni <pabeni@redhat.com> wrote:
>
>
>
> On 2/14/25 2:58 PM, Michal Swiatkowski wrote:
> > On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
> >> On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> >>> Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> >>> from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> >>> devlink_rel_alloc().
> >>
> >> If the same bug exists twice it might exist more times. Did you find
> >> this instance by searching the whole tree? Or just networking?
> >>
> >> This is also something which would be good to have the static
> >> analysers check for. I wounder if smatch can check this?
> >>
> >> Andrew
> >>
> >
> > You are right, I checked only net folder and there are two usage like
> > that in drivers. I will send v2 with wider fixing, thanks.
>
> While at that, please add the suitable fixes tag(s).
>
> Thanks,
>
> Paolo
Hello,
I haven't seen a v2 patch from Michal Swiatkowski. Would it be okay to
at least merge this net/devlink/core.c fix for inclusion in 6.14? I
can send a revised patch adding the Fixes tag. Driver fixes could be
addressed separately.
Thanks,
Pierre
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-03-10 11:42 ` Pierre Riteau
@ 2025-03-11 9:16 ` Michal Swiatkowski
2025-03-11 11:49 ` Dan Carpenter
0 siblings, 1 reply; 13+ messages in thread
From: Michal Swiatkowski @ 2025-03-11 9:16 UTC (permalink / raw)
To: Pierre Riteau
Cc: Paolo Abeni, Michal Swiatkowski, Andrew Lunn, netdev, jiri, davem,
edumazet, kuba, horms, Dan Carpenter
On Mon, Mar 10, 2025 at 12:42:13PM +0100, Pierre Riteau wrote:
> On Tue, 18 Feb 2025 at 12:56, Paolo Abeni <pabeni@redhat.com> wrote:
> >
> >
> >
> > On 2/14/25 2:58 PM, Michal Swiatkowski wrote:
> > > On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
> > >> On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> > >>> Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> > >>> from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> > >>> devlink_rel_alloc().
> > >>
> > >> If the same bug exists twice it might exist more times. Did you find
> > >> this instance by searching the whole tree? Or just networking?
> > >>
> > >> This is also something which would be good to have the static
> > >> analysers check for. I wounder if smatch can check this?
> > >>
> > >> Andrew
> > >>
> > >
> > > You are right, I checked only net folder and there are two usage like
> > > that in drivers. I will send v2 with wider fixing, thanks.
> >
> > While at that, please add the suitable fixes tag(s).
> >
> > Thanks,
> >
> > Paolo
>
> Hello,
>
> I haven't seen a v2 patch from Michal Swiatkowski. Would it be okay to
> at least merge this net/devlink/core.c fix for inclusion in 6.14? I
> can send a revised patch adding the Fixes tag. Driver fixes could be
> addressed separately.
>
Sorry that I didn't send v2, but I have seen that Dan wrote to Jiri
about this code and also found more places to fix. I assumed that he
will send a fix for all cases that he found.
Dan, do you plan to send it or I should send v2?
Thanks,
Michal
> Thanks,
> Pierre
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-03-11 9:16 ` Michal Swiatkowski
@ 2025-03-11 11:49 ` Dan Carpenter
2025-03-11 12:09 ` Michal Swiatkowski
0 siblings, 1 reply; 13+ messages in thread
From: Dan Carpenter @ 2025-03-11 11:49 UTC (permalink / raw)
To: Michal Swiatkowski
Cc: Pierre Riteau, Paolo Abeni, Andrew Lunn, netdev, jiri, davem,
edumazet, kuba, horms, Dan Carpenter
On Tue, Mar 11, 2025 at 10:16:55AM +0100, Michal Swiatkowski wrote:
> On Mon, Mar 10, 2025 at 12:42:13PM +0100, Pierre Riteau wrote:
> > On Tue, 18 Feb 2025 at 12:56, Paolo Abeni <pabeni@redhat.com> wrote:
> > >
> > >
> > >
> > > On 2/14/25 2:58 PM, Michal Swiatkowski wrote:
> > > > On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
> > > >> On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> > > >>> Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> > > >>> from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> > > >>> devlink_rel_alloc().
> > > >>
> > > >> If the same bug exists twice it might exist more times. Did you find
> > > >> this instance by searching the whole tree? Or just networking?
> > > >>
> > > >> This is also something which would be good to have the static
> > > >> analysers check for. I wounder if smatch can check this?
> > > >>
> > > >> Andrew
> > > >>
> > > >
> > > > You are right, I checked only net folder and there are two usage like
> > > > that in drivers. I will send v2 with wider fixing, thanks.
> > >
> > > While at that, please add the suitable fixes tag(s).
> > >
> > > Thanks,
> > >
> > > Paolo
> >
> > Hello,
> >
> > I haven't seen a v2 patch from Michal Swiatkowski. Would it be okay to
> > at least merge this net/devlink/core.c fix for inclusion in 6.14? I
> > can send a revised patch adding the Fixes tag. Driver fixes could be
> > addressed separately.
> >
>
> Sorry that I didn't send v2, but I have seen that Dan wrote to Jiri
> about this code and also found more places to fix. I assumed that he
> will send a fix for all cases that he found.
>
> Dan, do you plan to send it or I should send v2?
Sorry, no I didn't realize anyone was waiting for me on this. Could
you send it?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [net v1] devlink: fix xa_alloc_cyclic error handling
2025-03-11 11:49 ` Dan Carpenter
@ 2025-03-11 12:09 ` Michal Swiatkowski
0 siblings, 0 replies; 13+ messages in thread
From: Michal Swiatkowski @ 2025-03-11 12:09 UTC (permalink / raw)
To: Dan Carpenter
Cc: Michal Swiatkowski, Pierre Riteau, Paolo Abeni, Andrew Lunn,
netdev, jiri, davem, edumazet, kuba, horms, Dan Carpenter
On Tue, Mar 11, 2025 at 02:49:43PM +0300, Dan Carpenter wrote:
> On Tue, Mar 11, 2025 at 10:16:55AM +0100, Michal Swiatkowski wrote:
> > On Mon, Mar 10, 2025 at 12:42:13PM +0100, Pierre Riteau wrote:
> > > On Tue, 18 Feb 2025 at 12:56, Paolo Abeni <pabeni@redhat.com> wrote:
> > > >
> > > >
> > > >
> > > > On 2/14/25 2:58 PM, Michal Swiatkowski wrote:
> > > > > On Fri, Feb 14, 2025 at 02:44:49PM +0100, Andrew Lunn wrote:
> > > > >> On Fri, Feb 14, 2025 at 02:24:53PM +0100, Michal Swiatkowski wrote:
> > > > >>> Pierre Riteau <pierre@stackhpc.com> found suspicious handling an error
> > > > >>> from xa_alloc_cyclic() in scheduler code [1]. The same is done in
> > > > >>> devlink_rel_alloc().
> > > > >>
> > > > >> If the same bug exists twice it might exist more times. Did you find
> > > > >> this instance by searching the whole tree? Or just networking?
> > > > >>
> > > > >> This is also something which would be good to have the static
> > > > >> analysers check for. I wounder if smatch can check this?
> > > > >>
> > > > >> Andrew
> > > > >>
> > > > >
> > > > > You are right, I checked only net folder and there are two usage like
> > > > > that in drivers. I will send v2 with wider fixing, thanks.
> > > >
> > > > While at that, please add the suitable fixes tag(s).
> > > >
> > > > Thanks,
> > > >
> > > > Paolo
> > >
> > > Hello,
> > >
> > > I haven't seen a v2 patch from Michal Swiatkowski. Would it be okay to
> > > at least merge this net/devlink/core.c fix for inclusion in 6.14? I
> > > can send a revised patch adding the Fixes tag. Driver fixes could be
> > > addressed separately.
> > >
> >
> > Sorry that I didn't send v2, but I have seen that Dan wrote to Jiri
> > about this code and also found more places to fix. I assumed that he
> > will send a fix for all cases that he found.
> >
> > Dan, do you plan to send it or I should send v2?
>
> Sorry, no I didn't realize anyone was waiting for me on this. Could
> you send it?
>
Sure, I will do it. Thanks for clarification.
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-03-11 12:12 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-14 13:24 [net v1] devlink: fix xa_alloc_cyclic error handling Michal Swiatkowski
2025-02-14 13:44 ` Andrew Lunn
2025-02-14 13:58 ` Michal Swiatkowski
2025-02-14 14:14 ` Andrew Lunn
2025-02-18 11:56 ` Paolo Abeni
2025-03-10 11:42 ` Pierre Riteau
2025-03-11 9:16 ` Michal Swiatkowski
2025-03-11 11:49 ` Dan Carpenter
2025-03-11 12:09 ` Michal Swiatkowski
2025-02-16 15:06 ` Dan Carpenter
2025-02-16 16:08 ` Andrew Lunn
2025-02-17 7:46 ` Dan Carpenter
2025-02-17 6:57 ` Dan Carpenter
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).