All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Linkui Xiao <xiaolinkui@126.com>
Cc: jiri@resnulli.us, davem@davemloft.net, edumazet@kernel.org,
	kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linkui Xiao <xiaolinkui@kylinos.cn>
Subject: Re: [PATCH net 0/2] devlink: do not dereference a missing param item or shared buffer
Date: Tue, 3 Nov 2026 10:46:21 +0000	[thread overview]
Message-ID: <20261103104621.GD865646@horms.kernel.org> (raw)
In-Reply-To: <84706e2d-5196-4e40-8bf5-122a0c54eada@126.com>

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.

  reply	other threads:[~2026-11-03 10:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-30  3:33 [PATCH net 0/2] devlink: do not dereference a missing param item or shared buffer Linkui Xiao
2026-10-30  3:33 ` [PATCH net 1/2] devlink: param: do not dereference a missing param item Linkui Xiao
2026-11-02 12:02   ` Simon Horman
2026-10-30  3:33 ` [PATCH net 2/2] devlink: sb: do not dereference a missing shared buffer Linkui Xiao
2026-11-02 12:02   ` Simon Horman
2026-11-02 12:05 ` [PATCH net 0/2] devlink: do not dereference a missing param item or " Simon Horman
2026-11-02 13:12   ` Linkui Xiao
2026-11-03 10:46     ` Simon Horman [this message]
  -- strict thread matches above, loose matches on Subject: below --
2026-10-30  3:30 Linkui Xiao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261103104621.GD865646@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=xiaolinkui@126.com \
    --cc=xiaolinkui@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.