Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

skbuff: Introduce slab_build_skb()

syzkaller reported:

BUG: KASAN: slab-out-of-bounds in __build_skb_around+0x235/0x340 net/core/skbuff.c:294
Write of size 32 at addr ffff88802aa172c0 by task syz-executor413/5295

For bpf_prog_test_run_skb(), which uses a kmalloc()ed buffer passed to
build_skb().

When build_skb() is passed a frag_size of 0, it means the buffer came
from kmalloc. In these cases, ksize() is used to find its actual size,
but since the allocation may not have been made to that size, actually
perform the krealloc() call so that all the associated buffer size
checking will be correctly notified (and use the "new" pointer so that
compiler hinting works correctly). Split this logic out into a new
interface, slab_build_skb(), but leave the original 0 checking for now
to catch any stragglers.

Reported-by: syzbot+fda18eaa8c12534ccb3b@syzkaller.appspotmail.com
Link: https://groups.google.com/g/syzkaller-bugs/c/UnIKxTtU5-0/m/-wbXinkgAQAJ
Fixes: 38931d8989b5 ("mm: Make ksize() a reporting-only function")
Cc: Pavel Begunkov <asml.silence@gmail.com>
Cc: pepsipu <soopthegoop@gmail.com>
Cc: syzbot+fda18eaa8c12534ccb3b@syzkaller.appspotmail.com
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: kasan-dev <kasan-dev@googlegroups.com>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: ast@kernel.org
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Hao Luo <haoluo@google.com>
Cc: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: jolsa@kernel.org
Cc: KP Singh <kpsingh@kernel.org>
Cc: martin.lau@linux.dev
Cc: Stanislav Fomichev <sdf@google.com>
Cc: song@kernel.org
Cc: Yonghong Song <yhs@fb.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20221208060256.give.994-kees@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Kees Cook and committed by
Jakub Kicinski
ce098da1 28d39503

+66 -11
+1 -1
drivers/net/ethernet/broadcom/bnx2.c
··· 3045 3045 3046 3046 dma_unmap_single(&bp->pdev->dev, dma_addr, bp->rx_buf_use_size, 3047 3047 DMA_FROM_DEVICE); 3048 - skb = build_skb(data, 0); 3048 + skb = slab_build_skb(data); 3049 3049 if (!skb) { 3050 3050 kfree(data); 3051 3051 goto error;
+1 -1
drivers/net/ethernet/qlogic/qed/qed_ll2.c
··· 200 200 dma_unmap_single(&cdev->pdev->dev, buffer->phys_addr, 201 201 cdev->ll2->rx_size, DMA_FROM_DEVICE); 202 202 203 - skb = build_skb(buffer->data, 0); 203 + skb = slab_build_skb(buffer->data); 204 204 if (!skb) { 205 205 DP_INFO(cdev, "Failed to build SKB\n"); 206 206 kfree(buffer->data);
+1
include/linux/skbuff.h
··· 1255 1255 void skb_attempt_defer_free(struct sk_buff *skb); 1256 1256 1257 1257 struct sk_buff *napi_build_skb(void *data, unsigned int frag_size); 1258 + struct sk_buff *slab_build_skb(void *data); 1258 1259 1259 1260 /** 1260 1261 * alloc_skb - allocate a network buffer
+1 -1
net/bpf/test_run.c
··· 1128 1128 } 1129 1129 sock_init_data(NULL, sk); 1130 1130 1131 - skb = build_skb(data, 0); 1131 + skb = slab_build_skb(data); 1132 1132 if (!skb) { 1133 1133 kfree(data); 1134 1134 kfree(ctx);
+62 -8
net/core/skbuff.c
··· 270 270 return skb; 271 271 } 272 272 273 - /* Caller must provide SKB that is memset cleared */ 274 - static void __build_skb_around(struct sk_buff *skb, void *data, 275 - unsigned int frag_size) 273 + static inline void __finalize_skb_around(struct sk_buff *skb, void *data, 274 + unsigned int size) 276 275 { 277 276 struct skb_shared_info *shinfo; 278 - unsigned int size = frag_size ? : ksize(data); 279 277 280 278 size -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); 281 279 ··· 295 297 skb_set_kcov_handle(skb, kcov_common_handle()); 296 298 } 297 299 300 + static inline void *__slab_build_skb(struct sk_buff *skb, void *data, 301 + unsigned int *size) 302 + { 303 + void *resized; 304 + 305 + /* Must find the allocation size (and grow it to match). */ 306 + *size = ksize(data); 307 + /* krealloc() will immediately return "data" when 308 + * "ksize(data)" is requested: it is the existing upper 309 + * bounds. As a result, GFP_ATOMIC will be ignored. Note 310 + * that this "new" pointer needs to be passed back to the 311 + * caller for use so the __alloc_size hinting will be 312 + * tracked correctly. 313 + */ 314 + resized = krealloc(data, *size, GFP_ATOMIC); 315 + WARN_ON_ONCE(resized != data); 316 + return resized; 317 + } 318 + 319 + /* build_skb() variant which can operate on slab buffers. 320 + * Note that this should be used sparingly as slab buffers 321 + * cannot be combined efficiently by GRO! 322 + */ 323 + struct sk_buff *slab_build_skb(void *data) 324 + { 325 + struct sk_buff *skb; 326 + unsigned int size; 327 + 328 + skb = kmem_cache_alloc(skbuff_head_cache, GFP_ATOMIC); 329 + if (unlikely(!skb)) 330 + return NULL; 331 + 332 + memset(skb, 0, offsetof(struct sk_buff, tail)); 333 + data = __slab_build_skb(skb, data, &size); 334 + __finalize_skb_around(skb, data, size); 335 + 336 + return skb; 337 + } 338 + EXPORT_SYMBOL(slab_build_skb); 339 + 340 + /* Caller must provide SKB that is memset cleared */ 341 + static void __build_skb_around(struct sk_buff *skb, void *data, 342 + unsigned int frag_size) 343 + { 344 + unsigned int size = frag_size; 345 + 346 + /* frag_size == 0 is considered deprecated now. Callers 347 + * using slab buffer should use slab_build_skb() instead. 348 + */ 349 + if (WARN_ONCE(size == 0, "Use slab_build_skb() instead")) 350 + data = __slab_build_skb(skb, data, &size); 351 + 352 + __finalize_skb_around(skb, data, size); 353 + } 354 + 298 355 /** 299 356 * __build_skb - build a network buffer 300 357 * @data: data buffer provided by caller 301 - * @frag_size: size of data, or 0 if head was kmalloced 358 + * @frag_size: size of data (must not be 0) 302 359 * 303 360 * Allocate a new &sk_buff. Caller provides space holding head and 304 - * skb_shared_info. @data must have been allocated by kmalloc() only if 305 - * @frag_size is 0, otherwise data should come from the page allocator 306 - * or vmalloc() 361 + * skb_shared_info. @data must have been allocated from the page 362 + * allocator or vmalloc(). (A @frag_size of 0 to indicate a kmalloc() 363 + * allocation is deprecated, and callers should use slab_build_skb() 364 + * instead.) 307 365 * The return is the new skb buffer. 308 366 * On a failure the return is %NULL, and @data is not freed. 309 367 * Notes :