[PATCH net 0/2] devlink: do not dereference a missing param item or shared buffer

[PATCH net 0/2] devlink: do not dereference a missing param item or shared buffer

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

Two devlink helpers look an object up by id or index, warn when the
lookup fails, and then pass the NULL result to code that dereferences
it. A driver mistake (a param_id that was never registered, an sb_index
that was never registered or is unregistered twice) therefore costs an
oops instead of a warning.

  1/2 param: devl_param_value_changed() hands the missing param item to
      devlink_param_notify() -> devlink_nl_param_fill(), whose first
      statement dereferences param_item->param.

  2/2 sb: devl_sb_unregister() runs list_del() on the missing shared
      buffer; list is the first member of struct devlink_sb, so it
      reads and writes through address 0.

In both cases every other user of the same lookup already returns right
after the warning, so the fixes just align the remaining callers; there
is no change of behavior beyond avoiding the dereference. The param side
follows the example Jiri set for devlink_param_unregister():

Link: https://lore.kernel.org/all/20230126075838.1643665-5-jiri@resnulli.us/

Both Fixes: tags point at the commits that introduced the unguarded
WARN_ON pattern; the current devl_() functions are pure renames of those
originals. No in-tree caller triggers either path today (the audit is
part of a defensive hardening pass), so both are aimed at stopping
out-of-tree drivers from turning a driver bug into an oops.

Linkui Xiao (2):
  devlink: param: do not dereference a missing param item
  devlink: sb: do not dereference a missing shared buffer

 net/devlink/param.c | 3 ++-
 net/devlink/sb.c    | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH net 1/2] devlink: param: do not dereference a missing param item

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

devl_param_value_changed() looks the param item up by id and only warns
when the lookup fails. devlink_param_notify() is called with the result
regardless, and devlink_nl_param_fill() dereferences param_item->param
in its very first statement, so a driver that passes a param_id which is
not registered on this instance turns the warning into a NULL pointer
dereference.

Every other user of devlink_param_find_by_id() handles the NULL result:
devlink_param_unregister() and devl_param_driverinit_value_set() return
right after the same warning, devl_param_driverinit_value_get() returns
-EINVAL. Do the same here so that a bogus param_id costs a warning
instead of an oops.

Fixes: ea601e170988 ("devlink: Add devlink notifications support for params")
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
Link: https://lore.kernel.org/all/20230126075838.1643665-5-jiri@resnulli.us/
---
 net/devlink/param.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/devlink/param.c b/net/devlink/param.c
index 95ac9db8c993..474fe009f22d 100644
--- a/net/devlink/param.c
+++ b/net/devlink/param.c
@@ -1008,7 +1008,8 @@ void devl_param_value_changed(struct devlink *devlink, u32 param_id)
 	struct devlink_param_item *param_item;
 
 	param_item = devlink_param_find_by_id(&devlink->params, param_id);
-	WARN_ON(!param_item);
+	if (WARN_ON(!param_item))
+		return;
 
 	devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_NEW);
 }
-- 
2.25.1

Re: [PATCH net 1/2] devlink: param: do not dereference a missing param item

[Simon Horman]

On Fri, Oct 30, 2026 at 11:33:31AM +0800, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> devl_param_value_changed() looks the param item up by id and only warns
> when the lookup fails. devlink_param_notify() is called with the result
> regardless, and devlink_nl_param_fill() dereferences param_item->param
> in its very first statement, so a driver that passes a param_id which is
> not registered on this instance turns the warning into a NULL pointer
> dereference.
> 
> Every other user of devlink_param_find_by_id() handles the NULL result:
> devlink_param_unregister() and devl_param_driverinit_value_set() return
> right after the same warning, devl_param_driverinit_value_get() returns
> -EINVAL. Do the same here so that a bogus param_id costs a warning
> instead of an oops.
> 
> Fixes: ea601e170988 ("devlink: Add devlink notifications support for params")
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> Link: https://lore.kernel.org/all/20230126075838.1643665-5-jiri@resnulli.us/

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH net 2/2] devlink: sb: do not dereference a missing shared buffer

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

devl_sb_unregister() looks the shared buffer up by index and only warns
when it is not on the list. list_del() and kfree() are then called on the
NULL result, so unregistering an index that was never registered, or
unregistering the same index twice, turns the warning into a NULL pointer
dereference. list is the first member of struct devlink_sb, so list_del()
reads the next and prev pointers from address 0 before storing to them.

The register side already fails gracefully: devl_sb_register() returns
-EEXIST when the index is taken. Make the unregister side symmetric and
return after the warning.

Fixes: bf7974710a40 ("devlink: add shared buffer configuration")
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 net/devlink/sb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/devlink/sb.c b/net/devlink/sb.c
index 129bd016e302..4b444f2ca0f5 100644
--- a/net/devlink/sb.c
+++ b/net/devlink/sb.c
@@ -980,7 +980,8 @@ void devl_sb_unregister(struct devlink *devlink, unsigned int sb_index)
 	lockdep_assert_held(&devlink->lock);
 
 	devlink_sb = devlink_sb_get_by_index(devlink, sb_index);
-	WARN_ON(!devlink_sb);
+	if (WARN_ON(!devlink_sb))
+		return;
 	list_del(&devlink_sb->list);
 	kfree(devlink_sb);
 }
-- 
2.25.1

Re: [PATCH net 2/2] devlink: sb: do not dereference a missing shared buffer

[Simon Horman]

On Fri, Oct 30, 2026 at 11:33:32AM +0800, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> devl_sb_unregister() looks the shared buffer up by index and only warns
> when it is not on the list. list_del() and kfree() are then called on the
> NULL result, so unregistering an index that was never registered, or
> unregistering the same index twice, turns the warning into a NULL pointer
> dereference. list is the first member of struct devlink_sb, so list_del()
> reads the next and prev pointers from address 0 before storing to them.
> 
> The register side already fails gracefully: devl_sb_register() returns
> -EEXIST when the index is taken. Make the unregister side symmetric and
> return after the warning.
> 
> Fixes: bf7974710a40 ("devlink: add shared buffer configuration")
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net 0/2] devlink: do not dereference a missing param item or shared buffer

[Simon Horman]

On Fri, Oct 30, 2026 at 11:33:30AM +0800, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> Two devlink helpers look an object up by id or index, warn when the
> lookup fails, and then pass the NULL result to code that dereferences
> it. A driver mistake (a param_id that was never registered, an sb_index
> that was never registered or is unregistered twice) therefore costs an
> oops instead of a warning.
> 
>   1/2 param: devl_param_value_changed() hands the missing param item to
>       devlink_param_notify() -> devlink_nl_param_fill(), whose first
>       statement dereferences param_item->param.
> 
>   2/2 sb: devl_sb_unregister() runs list_del() on the missing shared
>       buffer; list is the first member of struct devlink_sb, so it
>       reads and writes through address 0.
> 
> In both cases every other user of the same lookup already returns right
> after the warning, so the fixes just align the remaining callers; there
> is no change of behavior beyond avoiding the dereference. The param side
> follows the example Jiri set for devlink_param_unregister():
> 
> Link: https://lore.kernel.org/all/20230126075838.1643665-5-jiri@resnulli.us/
> 
> Both Fixes: tags point at the commits that introduced the unguarded
> WARN_ON pattern; the current devl_() functions are pure renames of those
> originals. No in-tree caller triggers either path today (the audit is
> part of a defensive hardening pass), so both are aimed at stopping
> out-of-tree drivers from turning a driver bug into an oops.

Thanks, both these fixes look good to me.

But I do wonder if the hardening aspect, from an in-tree pov,
makes them net-next rather than net material.

I say that, because my understanding is that there is some pressure
to only push critical bug fixes via net. This is because of the recent
up-tick in bug fixes driven by AI-related activities.

Re: [PATCH net 0/2] devlink: do not dereference a missing param item or shared buffer

[Linkui Xiao]

Hi Simon,

thanks for the review.

On 2026/11/2 20:05, Simon Horman wrote:
> On Fri, Oct 30, 2026 at 11:33:30AM +0800, Linkui Xiao wrote:
>> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>>
>> Two devlink helpers look an object up by id or index, warn when the
>> lookup fails, and then pass the NULL result to code that dereferences
>> it. A driver mistake (a param_id that was never registered, an sb_index
>> that was never registered or is unregistered twice) therefore costs an
>> oops instead of a warning.
>>
>>    1/2 param: devl_param_value_changed() hands the missing param item to
>>        devlink_param_notify() -> devlink_nl_param_fill(), whose first
>>        statement dereferences param_item->param.
>>
>>    2/2 sb: devl_sb_unregister() runs list_del() on the missing shared
>>        buffer; list is the first member of struct devlink_sb, so it
>>        reads and writes through address 0.
>>
>> In both cases every other user of the same lookup already returns right
>> after the warning, so the fixes just align the remaining callers; there
>> is no change of behavior beyond avoiding the dereference. The param side
>> follows the example Jiri set for devlink_param_unregister():
>>
>> Link: https://lore.kernel.org/all/20230126075838.1643665-5-jiri@resnulli.us/
>>
>> Both Fixes: tags point at the commits that introduced the unguarded
>> WARN_ON pattern; the current devl_() functions are pure renames of those
>> originals. No in-tree caller triggers either path today (the audit is
>> part of a defensive hardening pass), so both are aimed at stopping
>> out-of-tree drivers from turning a driver bug into an oops.
> 
> Thanks, both these fixes look good to me.
> 
> But I do wonder if the hardening aspect, from an in-tree pov,
> makes them net-next rather than net material.
> 
> I say that, because my understanding is that there is some pressure
> to only push critical bug fixes via net. This is because of the recent
> up-tick in bug fixes driven by AI-related activities.

Agreed -- net-next is open now, so I have just resent this as
[PATCH net-next v2 0/2]. Only the prefix and the base changed: the two 
hunks,
the Fixes: tags and the commit messages are the ones you reviewed, and the
Reviewed-by tags from v1 are kept. Neither patch carries a Cc: stable, so
there is nothing to drop with the new target.

Thanks,
Linkui

Re: [PATCH net 0/2] devlink: do not dereference a missing param item or shared buffer

[Simon Horman]

On Mon, Nov 02, 2026 at 09:12:16PM +0800, Linkui Xiao wrote:
> Hi Simon,
> 
> thanks for the review.
> 
> On 2026/11/2 20:05, Simon Horman wrote:
> > On Fri, Oct 30, 2026 at 11:33:30AM +0800, Linkui Xiao wrote:
> > > From: Linkui Xiao <xiaolinkui@kylinos.cn>
> > > 
> > > Two devlink helpers look an object up by id or index, warn when the
> > > lookup fails, and then pass the NULL result to code that dereferences
> > > it. A driver mistake (a param_id that was never registered, an sb_index
> > > that was never registered or is unregistered twice) therefore costs an
> > > oops instead of a warning.
> > > 
> > >    1/2 param: devl_param_value_changed() hands the missing param item to
> > >        devlink_param_notify() -> devlink_nl_param_fill(), whose first
> > >        statement dereferences param_item->param.
> > > 
> > >    2/2 sb: devl_sb_unregister() runs list_del() on the missing shared
> > >        buffer; list is the first member of struct devlink_sb, so it
> > >        reads and writes through address 0.
> > > 
> > > In both cases every other user of the same lookup already returns right
> > > after the warning, so the fixes just align the remaining callers; there
> > > is no change of behavior beyond avoiding the dereference. The param side
> > > follows the example Jiri set for devlink_param_unregister():
> > > 
> > > Link: https://lore.kernel.org/all/20230126075838.1643665-5-jiri@resnulli.us/
> > > 
> > > Both Fixes: tags point at the commits that introduced the unguarded
> > > WARN_ON pattern; the current devl_() functions are pure renames of those
> > > originals. No in-tree caller triggers either path today (the audit is
> > > part of a defensive hardening pass), so both are aimed at stopping
> > > out-of-tree drivers from turning a driver bug into an oops.
> > 
> > Thanks, both these fixes look good to me.
> > 
> > But I do wonder if the hardening aspect, from an in-tree pov,
> > makes them net-next rather than net material.
> > 
> > I say that, because my understanding is that there is some pressure
> > to only push critical bug fixes via net. This is because of the recent
> > up-tick in bug fixes driven by AI-related activities.
> 
> Agreed -- net-next is open now, so I have just resent this as
> [PATCH net-next v2 0/2]. Only the prefix and the base changed: the two
> hunks,
> the Fixes: tags and the commit messages are the ones you reviewed, and the
> Reviewed-by tags from v1 are kept. Neither patch carries a Cc: stable, so
> there is nothing to drop with the new target.

Thanks, much appreciated.