Re: How do I avoid recvmsg races with IP_RECVERR?

[Hannes Frederic Sowa <hannes@stressinduktion.org>]

On Tue, Jun 2, 2015, at 23:42, Hannes Frederic Sowa wrote:
> On Tue, Jun 2, 2015, at 23:33, Andy Lutomirski wrote:
> > On Tue, Jun 2, 2015 at 2:17 PM, Hannes Frederic Sowa
> > <hannes@stressinduktion.org> wrote:
> > > On Tue, Jun 2, 2015, at 21:40, Andy Lutomirski wrote:
> >
> > [...]
> >
> > I do this already, which makes me think that there's a bug or another
> > race somewhere.  I've only seen a failure once in several years of
> > operation.
> > 
> > The failure happened on a ping socket.  I suspect that the race is:
> > 
> > ping_err: ip_icmp_error(...);
> > 
> > user: recvmsg(MSG_ERRQUEUE) and dequeues the error.
> > 
> > ping_err: sk_err = err;
> > 
> > user: recvmsg(MSG_ERRQUEUE not set), and recvmsg sees and clears the
> > error via sock_error.
> > 
> > user: recvmsg(MSG_ERRQUEUE), and recvmsg returns -EAGAIN.
> > 
> > Now the user code thinks that it was a real (non-transient) error and
> > aborts.
> > 
> > Shouldn't that sk->sk_err = err assignment at least use WRITE_ONCE?
> 
> Hmm, I don't think this will help.
> 
> > Even if this race were fixed, this interface still sucks IMO.
> 
> Yes. :/
> 
> My proposal would be to make the error conversion lazy:
> 
> Keeping duplicate data is not a good idea in general: So we shouldn't
> use sk->sk_err if IP_RECVERR is set at all but let sock_error just use
> the sk_error_queue and extract the error code from there.
> 
> Only if IP_RECVERR was not set, we use sk->sk_err logic.
> 
> What do you think?

I just noticed that this will probably break existing user space
applications which require that icmp errors are transient even with
IP_RECVERR. We can mark that with a bit in the sk_error_queue pointer
and xchg the pointer, hmmm....

Bye,
Hannes