Re: [PATCH net] team: fix possible deadlock in team_port_change_check
From: Jeongjun Park
Date: Thu Aug 01 2024 - 18:51:42 EST
Jakub Kicinski wrote:
>
> On Thu, 1 Aug 2024 20:18:42 +0900 Jeongjun Park wrote:
> > struct team *team = port->team;
> > + bool flag = true;
> >
> > - mutex_lock(&team->lock);
> > + if (mutex_is_locked(&team->lock)){
> > + unsigned long owner, curr = (unsigned long)current;
> > + owner = atomic_long_read(&team->lock.owner);
> > + if (owner != curr)
> > + mutex_lock(&team->lock);
> > + else
> > + flag = false;
> > + }
> > + else{
> > + mutex_lock(&team->lock);
> > + }
> > __team_port_change_check(port, linkup);
> > - mutex_unlock(&team->lock);
> > + if (flag)
> > + mutex_unlock(&team->lock);
>
> You didn't even run this thru checkpatch, let alone the fact that its
> reimplementing nested locks (or trying to) :(
>
> Some of the syzbot reports are not fixed because they are either hard
> or because there is a long standing disagreement on how to solve them.
> Please keep that in mind.
Okay, but I have a question. Is it true that team devices can also be
protected through rtnl? As far as I know, rtnl only protects net_device,
so I didn't think about removing the lock for team->lock.
Regards,
Jeongjun Park