netdev - Re: [PATCH net-next] sock: consistent errqueue errors and signals

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 2 Sep 2014 11:20:34 -0400
From:	Willem de Bruijn <willemb@...gle.com>
To:	Hannes Frederic Sowa <hannes@...hat.com>
Cc:	Network Development <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>
Subject: Re: [PATCH net-next] sock: consistent errqueue errors and signals

> From my experience in IPv6 code, we only do sk->sk_err updates directly
> in protocol error handling code. In case of UDP IPv6 errors for example
> we now notify sk_error_report two times with this patch (before the
> patch we did sk_data_ready (this is what you changed) and
> sk_error_report).

If the event is that an error is ready, is this not correct? The
wake up key should be POLLERR in both cases. In implementation
of sock_def_error_report and sock_def_readable, the difference
otherwise seems slim. I haven't checked all sk_data_ready and
sk_error_report implementations, though, so may have missed
differences for specific protocols. If this is not as obviously a strict
improvement as I thought, I'll just drop it.

> I really wonder if setting sk->sk_err in this function is the right
> thing to do.

I agree, in that it is hard to verify that this does not overwrite
an existing error. This patch only makes the behavior
consistent between enqueue and dequeue, but perhaps a
better way to achieve that is to change the dequeue side:
remove the assignment to sk->sk_err there. If so, then all
locations that currently check the state of sk->sk_err should
be changed to also check the qlen of the error queue and
if non-zero return the embedded error of the first skb. I'll
take a look whether that is feasible without adding locks
or atomics in the common path.

> It also depends on socket state bits (e.g. np->recverr) if
> the update happens. So we still cannot get rid of the protocol dependent
> sk->sk_err updates.
>
> It looks like we have to check all error handling functions in the
> protocols. Maybe timestamp code needs to adapt?

Does the above sound okay, or did you mean something else?

>
> Thanks,
> 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