Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PROPOSAL] Memory allocation for debugging #2211

Open
kingluo opened this issue Aug 13, 2024 · 3 comments
Open

[PROPOSAL] Memory allocation for debugging #2211

kingluo opened this issue Aug 13, 2024 · 3 comments
Milestone

Comments

@kingluo
Copy link
Contributor

kingluo commented Aug 13, 2024

As known, we use our home-brewed memory allocation for performance, e.g. tfw_pool_new, but the drawback is that it bypasses the KASAN. So we must ensure memory safety ourselves and cannot rely on general sanitizers.

  1. we don't use kmalloc/kmem_cache, so KASAN does not shadow the heap memory.
  2. some memory manipulation functions e.g. memcpy are written in assembly code by ourselves, so no asan instrumentation is generated by GCC, as well as some overflow checks (through some compiler tricks e.g. __builtin_object_size).

If we forge the same bug like #2208 but using kmalloc and the kernel's memcpy:

  void foobar(void)
  {
          const int sz = sizeof(TfwStr)*2;
          TfwStr* dst = kmalloc(sz*2, GFP_ATOMIC);
          TfwStr* src = kmalloc(sz, GFP_ATOMIC);
          memcpy(dst+sz, src, sz);   // <--------------- wrong pointer arithmetic
  }

Then KASAN will report it, we can explicitly know the bug.

[  272.552194] ==================================================================
[  272.552286] BUG: KASAN: slab-out-of-bounds in foobar+0x65/0x70 [tempesta_fw]
[  272.552343] Write of size 64 at addr ffff888106ac0f00 by task h2/2109

[  272.552414] CPU: 2 PID: 2109 Comm: h2 Kdump: loaded Tainted: G           OE     5.10.35+ #1
[  272.552480] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[  272.552567] Call Trace:
[  272.552590]  <IRQ>
[  272.552614]  dump_stack+0x96/0xc4
[  272.552679]  print_address_description.constprop.0+0x21/0x220
[  272.552728]  ? _raw_spin_lock_irqsave+0x8e/0xf0
[  272.552765]  ? _raw_write_unlock_bh+0x30/0x30
[  272.552817]  ? foobar+0x65/0x70 [tempesta_fw]
[  272.552855]  kasan_report.cold+0x20/0x37
[  272.552903]  ? foobar+0x65/0x70 [tempesta_fw]
[  272.552940]  check_memory_region+0x14d/0x1a0
[  272.552976]  memcpy+0x3c/0x60
[  272.553061]  foobar+0x65/0x70 [tempesta_fw]

We should make a unified API to manage memory but with different implementations for debug or release. We can enable the kernel's implementation automatically at the compile time if CONFIG_KASAN is 1.

@krizhanovsky
Copy link
Contributor

krizhanovsky commented Aug 13, 2024

This is a reliability issue. Relatively crucial since KASAN could reveal the bug #2208 , which took quite a time to find. Must be solved in 0.9.

The thing is that TfwPool is a pool allocator, which frees many items at once and there is no such allocator in the Linux kernel. So just using kmallc() instead could be problematic. Maybe the better way is to make TfwPool work with KASAN,

@RomanBelozerov probably we'll need a separate CI workflow, maybe periodic, to run all our test suite with as much as possible kernel sanitizers and checkers.

@krizhanovsky krizhanovsky added this to the 0.9 - LA milestone Aug 13, 2024
@krizhanovsky krizhanovsky added the good to start Start form this tasks if you're new in Tempesta FW label Aug 13, 2024
@krizhanovsky
Copy link
Contributor

One more thing: TfwPool-allocated pages can be moved to skb and sent as paged data, so using kmalloc() directly seems not possible.

Also would be good to make the system also work with kmemleak and other memory debugging tools

@krizhanovsky
Copy link
Contributor

The tool is required to have to reveal further bugs and regressions, but it does take significant development effort, so I have to move it to the next milestone

@krizhanovsky krizhanovsky modified the milestones: 0.9 - LA, 1.0 - GA Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants