Re: [PATCH net,v2] team: fix possible deadlock in team_port_change_check

[Jeongjun Park]

Jiri Pirko wrote:
>
> Sat, Aug 03, 2024 at 03:36:48AM CEST, aha310510@xxxxxxxxx wrote:
> >
> >
> >> Jiri Pirko wrote:
> >>
> >> Fri, Aug 02, 2024 at 06:25:31PM CEST, aha310510@xxxxxxxxx wrote:
> >>> Eric Dumazet wrote:
> >>>>
> >>>>> On Fri, Aug 2, 2024 at 5:00 PM Jeongjun Park <aha310510@xxxxxxxxx> 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@xxxxxxxxxx>
> 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