Re: [PATCH 4.19 03/54] sctp: fix refcount bug in sctp_wfree

[Marcelo Ricardo Leitner]

On Sat, Apr 11, 2020 at 08:28:13PM +0200, Pavel Machek wrote:
> Hi!
> 
> > The following case cause the bug:
> > for the trouble SKB, it was in outq->transmitted list
> 
> Ok... but this is one hell of "interesting" code.
> 
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -165,29 +165,44 @@ static void sctp_clear_owner_w(struct sc
> >  	skb_orphan(chunk->skb);
> >  }
> >  
> > +#define traverse_and_process()	\
> > +do {				\
> > +	msg = chunk->msg;	\
> > +	if (msg == prev_msg)	\
> > +		continue;	\
> > +	list_for_each_entry(c, &msg->chunks, frag_list) {	\
> > +		if ((clear && asoc->base.sk == c->skb->sk) ||	\
> > +		    (!clear && asoc->base.sk != c->skb->sk))	\
> > +			cb(c);	\
> > +	}			\
> > +	prev_msg = msg;		\
> > +} while (0)
> 
> Should this be function?
> 
> Do you see how it does "continue"? But the do {} while(0) wrapper eats
> the continue. "break" would be more clear here, but they are really
> equivalent due to way this macro is used.
> 
> It is just very, very confusing.

Agree. The 'continue' itself slipped in on a refactoring and I didn't
notice the confusing aspect of it. Initially, the code was written
without the macro, and the continue refererred to the outter
list_for_each_entry().

  Marcelo