On Wed, Aug 16, 2017 at 7:55 PM, Thiago Macieira
<[email protected]> wrote:
> On Wednesday, 16 August 2017 16:27:17 PDT Willem de Bruijn wrote:
>> Actually, it is safe even without the check. Overflow of the signed integer
>> is benign here.
>
> Usually, it's a bad idea to allow UB to happen.
>
> Where is the overflow? I didn't see it in the patches so far.
There isn't. Instead of using a shift, as I proposed earlier, the latest
patch just passes the raw value of sk_peek_off.
The shift was an attempt to disambiguate between the cases that
return zero in sk_peek_offset:
(a) peek without SO_PEEK_OFF:
each consecutive peek must look at the first packet
(b) peek with offset zero:
peek at the first packet, move forward with each call
After coding up a version that shifts by one to differentiate the two
cases of zero it was obvious that there is no need to shift at all:
the case without SO_PEEK_OFF is identified by all negative values
if sk_peek_offset just does not explicitly return 0 for this case.
Of course, then all callers need to be verified to correctly handle
negative values. __sk_try_recv_from_queue uses this information.
The only other cases, unix stream, does not care about the difference,
as the bytestream has no record separators.
What is UB in this context?