• Daniel Borkmann's avatar
    bpf: fix several offset tests in bpf_msg_pull_data · 5b24109b
    Daniel Borkmann authored
    While recently going over bpf_msg_pull_data(), I noticed three
    issues which are fixed in here:
    
    1) When we attempt to find the first scatterlist element (sge)
       for the start offset, we add len to the offset before we check
       for start < offset + len, whereas it should come after when
       we iterate to the next sge to accumulate the offsets. For
       example, given a start offset of 12 with a sge length of 8
       for the first sge in the list would lead us to determine this
       sge as the first sge thinking it covers first 16 bytes where
       start is located, whereas start sits in subsequent sges so
       we would end up pulling in the wrong data.
    
    2) After figuring out the starting sge, we have a short-cut test
       in !msg->sg_copy[i] && bytes <= len. This checks whether it's
       not needed to make the page at the sge private where we can
       just exit by updating msg->data and msg->data_end. However,
       the length test is not fully correct. bytes <= len checks
       whether the requested bytes (end - start offsets) fit into the
       sge's length. The part that is missing is that start must not
       be sge length aligned. Meaning, the start offset into the sge
       needs to be accounted as well on top of the requested bytes
       as otherwise we can access the sge out of bounds. For example
       the sge could have length of 8, our requested bytes could have
       length of 8, but at a start offset of 4, so we also would need
       to pull in 4 bytes of the next sge, when we jump to the out
       label we do set msg->data to sg_virt(&sg[i]) + start - offset
       and msg->data_end to msg->data + bytes which would be oob.
    
    3) The subsequent bytes < copy test for finding the last sge has
       the same issue as in point 2) but also it tests for less than
       rather than less or equal to. Meaning if the sge length is of
       8 and requested bytes of 8 while having the start aligned with
       the sge, we would unnecessarily go and pull in the next sge as
       well to make it private.
    
    Fixes: 015632bb ("bpf: sk_msg program helper bpf_sk_msg_pull_data")
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
    Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    5b24109b
filter.c 194 KB