| Message ID | 20210618075805.14412-1-magnus.karlsson@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f654fae47e83e56b454fbbfd0af0a4f232e356d6 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf,v2] xsk: fix broken Tx ring validation | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 9 maintainers not CCed: yhs@fb.com kpsingh@kernel.org hawk@kernel.org andrii@kernel.org kafai@fb.com john.fastabend@gmail.com songliubraving@fb.com davem@davemloft.net kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 86 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
On Fri, 18 Jun 2021 at 09:58, Magnus Karlsson <magnus.karlsson@gmail.com> wrote: > > From: Magnus Karlsson <magnus.karlsson@intel.com> > > Fix broken Tx ring validation for AF_XDP. The commit under the Fixes > tag, fixed an off-by-one error in the validation but introduced > another error. Descriptors are now let through even if they straddle a > chunk boundary which they are not allowed to do in aligned mode. Worse > is that they are let through even if they straddle the end of the umem > itself, tricking the kernel to read data outside the allowed umem > region which might or might not be mapped at all. > > Fix this by reintroducing the old code, but subtract the length by one > to fix the off-by-one error that the original patch was > addressing. The test chunk != chunk_end makes sure packets do not > straddle chunk boundraries. Note that packets of zero length are > allowed in the interface, therefore the test if the length is > non-zero. > > v1 -> v2: > * Improved commit message > > Fixes: ac31565c2193 ("xsk: Fix for xp_aligned_validate_desc() when len == chunk_size") > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Björn Töpel <bjorn@kernel.org> > --- > net/xdp/xsk_queue.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h > index 9d2a89d793c0..9ae13cccfb28 100644 > --- a/net/xdp/xsk_queue.h > +++ b/net/xdp/xsk_queue.h > @@ -128,12 +128,15 @@ static inline bool xskq_cons_read_addr_unchecked(struct xsk_queue *q, u64 *addr) > static inline bool xp_aligned_validate_desc(struct xsk_buff_pool *pool, > struct xdp_desc *desc) > { > - u64 chunk; > - > - if (desc->len > pool->chunk_size) > - return false; > + u64 chunk, chunk_end; > > chunk = xp_aligned_extract_addr(pool, desc->addr); > + if (likely(desc->len)) { > + chunk_end = xp_aligned_extract_addr(pool, desc->addr + desc->len - 1); > + if (chunk != chunk_end) > + return false; > + } > + > if (chunk >= pool->addrs_cnt) > return false; > > > base-commit: da5ac772cfe2a03058b0accfac03fad60c46c24d > -- > 2.29.0 >
Hello: This patch was applied to bpf/bpf.git (refs/heads/master): On Fri, 18 Jun 2021 09:58:05 +0200 you wrote: > From: Magnus Karlsson <magnus.karlsson@intel.com> > > Fix broken Tx ring validation for AF_XDP. The commit under the Fixes > tag, fixed an off-by-one error in the validation but introduced > another error. Descriptors are now let through even if they straddle a > chunk boundary which they are not allowed to do in aligned mode. Worse > is that they are let through even if they straddle the end of the umem > itself, tricking the kernel to read data outside the allowed umem > region which might or might not be mapped at all. > > [...] Here is the summary with links: - [bpf,v2] xsk: fix broken Tx ring validation https://git.kernel.org/bpf/bpf/c/f654fae47e83 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h index 9d2a89d793c0..9ae13cccfb28 100644 --- a/net/xdp/xsk_queue.h +++ b/net/xdp/xsk_queue.h @@ -128,12 +128,15 @@ static inline bool xskq_cons_read_addr_unchecked(struct xsk_queue *q, u64 *addr) static inline bool xp_aligned_validate_desc(struct xsk_buff_pool *pool, struct xdp_desc *desc) { - u64 chunk; - - if (desc->len > pool->chunk_size) - return false; + u64 chunk, chunk_end; chunk = xp_aligned_extract_addr(pool, desc->addr); + if (likely(desc->len)) { + chunk_end = xp_aligned_extract_addr(pool, desc->addr + desc->len - 1); + if (chunk != chunk_end) + return false; + } + if (chunk >= pool->addrs_cnt) return false;