[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1433281844.2487226.285170105.063B7084@webmail.messagingengine.com>
Date: Tue, 02 Jun 2015 23:50:44 +0200
From: Hannes Frederic Sowa <hannes@...essinduktion.org>
To: Andy Lutomirski <luto@...capital.net>
Cc: Andy Lutomirski <luto@...nel.org>,
Network Development <netdev@...r.kernel.org>
Subject: Re: How do I avoid recvmsg races with IP_RECVERR?
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@...essinduktion.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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists