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
