netdev - Re: [PATCH net,v2] team: fix possible deadlock in team_port_change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAO9qdTGLs8_1vi_s-87e-WOBmzzDEXQnx0eWcTwQibUBujiFug@mail.gmail.com>
Date: Sat, 3 Aug 2024 20:51:16 +0900
From: Jeongjun Park <aha310510@...il.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: edumazet@...gle.com, davem@...emloft.net, kuba@...nel.org, 
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org, pabeni@...hat.com, 
	syzbot+b668da2bc4cb9670bf58@...kaller.appspotmail.com
Subject: Re: [PATCH net,v2] team: fix possible deadlock in team_port_change_check

Jiri Pirko wrote:
>
> Sat, Aug 03, 2024 at 03:36:48AM CEST, aha310510@...il.com wrote:
> >
> >
> >> Jiri Pirko wrote:
> >>
> >> Fri, Aug 02, 2024 at 06:25:31PM CEST, aha310510@...il.com wrote:
> >>> Eric Dumazet wrote:
> >>>>
> >>>>> On Fri, Aug 2, 2024 at 5:00 PM Jeongjun Park <aha310510@...il.com> wrote:
> >>>>>>
> >>>
> >>> [..]
> >>>
> >>> @@ -2501,6 +2470,11 @@ int team_nl_options_get_doit(struct sk_buff *skb, struct genl_info *info)
> >>>    int err;
> >>>    LIST_HEAD(sel_opt_inst_list);
> >>>
> >>> +    if (!rtnl_is_locked()) {
> >>
> >> This is completely wrong, other thread may hold the lock.
> >>
> >>
> >>> +        rtnl_lock();
> >>
> >> NACK! I wrote it in the other thread. Don't take rtnl for get options
> >> command. It is used for repeated fetch of stats. It's read only. Should
> >> be converted to RCU.
> >>
> >
> >I see. But, in the current, when called through the following path:
> >team_nl_send_event_options_get()->
> >team_nl_send_options_get()->
> >team_nl_fill_one_option_get()
> >, it was protected through rtnl. Does this mean that rcu should be
>
> Not true. team_nl_send_event_options_get() is sometimes called without
> rtnl (from lb_stats_refresh() for example).
>
>
> >used instead of rtnl in this case as well?
> >
> >> Why are you so obsessed by this hypothetical syzcaller bug? Are you
> >> hitting this in real? If not, please let it go. I will fix it myself
> >> when I find some spare cycles.
> >
> >Sorry for the inconvenience, but I don't want to give up on this bug
> >so easily since it is a valid bug that we have started analyzing
> >anyway and the direction of how to fix it is clear. I hope you
> >understand and I will send you a patch that uses rcu instead
>
> I don't understand, sorry. Apparently you don't have any clue what you
> are doing and only waste people time. Can't you find another toy to
> play?
>
> Please add my
> Nacked-by: Jiri Pirko <jiri@...dia.com>
> tag to your any future attempt, as I'm sure it won't be correct.
>

I am truly sorry. I feel like I wasted people time by sending meaningless
patches repeatedly, approaching this issue recklessly. I will be more
careful in my analysis and sending emails in the future.

Regards,
Jeongjun Park