5:02 a.m.
Linux 5.7 and later kernels that contain kernel commit <1ad53d9fa3f6>
("slub: improve bit diffusion for freelist ptr obfuscation") changed
the calculation formula in the freelist_ptr(), which added a swab()
call to mix bits a little more. When kernel is built with the
"CONFIG_SLAB_FREELIST_HARDENED=y",the "kmem -s" option fails with the
following errors, if there is no such patch.
crash> kmem -s
CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
82166d00 144 0 0 0 4k fuse_request
82166e00 792 0 0 0 16k fuse_inode
87201e00 528 0 0 0 8k xfs_dqtrx
87201f00 496 0 0 0 8k xfs_dquot
kmem: xfs_buf: slab: 37202e6e900 invalid freepointer: b844bab900001d70
kmem: xfs_buf: slab: 3720250fd80 invalid freepointer: b8603f9400001370
...
Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com>
---
memory.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/memory.c b/memory.c
index 8c6bbe409922..a3cf8a86728d 100644
--- a/memory.c
+++ b/memory.c
@@ -20,6 +20,7 @@
#include <sys/mman.h>
#include <ctype.h>
#include <netinet/in.h>
+#include <byteswap.h>
struct meminfo { /* general purpose memory information structure */
ulong cache; /* used by the various memory searching/dumping */
@@ -19336,10 +19337,14 @@ count_free_objects(struct meminfo *si, ulong freelist)
static ulong
freelist_ptr(struct meminfo *si, ulong ptr, ulong ptr_addr)
{
- if (VALID_MEMBER(kmem_cache_random))
+ if (VALID_MEMBER(kmem_cache_random)) {
/* CONFIG_SLAB_FREELIST_HARDENED */
+
+ if (THIS_KERNEL_VERSION >= LINUX(5,7,0))
+ ptr_addr = (sizeof(long) == 8) ? bswap_64(ptr_addr)
+ : bswap_32(ptr_addr);
return (ptr ^ si->random ^ ptr_addr);
- else
+ } else
return ptr;
}
--
2.17.1
3:43 a.m.
Hi Lianbo,
-----Original Message-----
Linux 5.7 and later kernels that contain kernel commit
<1ad53d9fa3f6>
("slub: improve bit diffusion for freelist ptr obfuscation") changed
the calculation formula in the freelist_ptr(), which added a swab()
call to mix bits a little more. When kernel is built with the
"CONFIG_SLAB_FREELIST_HARDENED=y",the "kmem -s" option fails with
the
following errors, if there is no such patch.
crash> kmem -s
CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
82166d00 144 0 0 0 4k fuse_request
82166e00 792 0 0 0 16k fuse_inode
87201e00 528 0 0 0 8k xfs_dqtrx
87201f00 496 0 0 0 8k xfs_dquot
kmem: xfs_buf: slab: 37202e6e900 invalid freepointer: b844bab900001d70
kmem: xfs_buf: slab: 3720250fd80 invalid freepointer: b8603f9400001370
...
Good catch! And the patch diff looks good to me.
But the freelist_ptr() function, which is patched, is called only when
the error message is NOT printed. So it seems like the patch does not
stop the message, right?
static ulong
get_freepointer(struct meminfo *si, void *object)
{
ulong vaddr, nextfree;
vaddr = (ulong)(object + si->slab_offset);
if (!readmem(vaddr, KVADDR, &nextfree,
sizeof(void *), "get_freepointer", QUIET|RETURN_ON_ERROR)) {
error(INFO, "%s: slab: %lx invalid freepointer: %lx\n",
si->curname, si->slab, vaddr);
return BADADDR;
}
return (freelist_ptr(si, nextfree, vaddr));
}
If so, please fix the commit message. What is the phenomenon that
the patch fixes?
Thanks,
Kazu
Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com>
---
memory.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/memory.c b/memory.c
index 8c6bbe409922..a3cf8a86728d 100644
--- a/memory.c
+++ b/memory.c
@@ -20,6 +20,7 @@
#include <sys/mman.h>
#include <ctype.h>
#include <netinet/in.h>
+#include <byteswap.h>
struct meminfo { /* general purpose memory information structure */
ulong cache; /* used by the various memory searching/dumping */
@@ -19336,10 +19337,14 @@ count_free_objects(struct meminfo *si, ulong freelist)
static ulong
freelist_ptr(struct meminfo *si, ulong ptr, ulong ptr_addr)
{
- if (VALID_MEMBER(kmem_cache_random))
+ if (VALID_MEMBER(kmem_cache_random)) {
/* CONFIG_SLAB_FREELIST_HARDENED */
+
+ if (THIS_KERNEL_VERSION >= LINUX(5,7,0))
+ ptr_addr = (sizeof(long) == 8) ? bswap_64(ptr_addr)
+ : bswap_32(ptr_addr);
return (ptr ^ si->random ^ ptr_addr);
- else
+ } else
return ptr;
}
--
2.17.1
3:43 p.m.
-----Original Message-----
> Linux 5.7 and later kernels that contain kernel commit <1ad53d9fa3f6>
> ("slub: improve bit diffusion for freelist ptr obfuscation") changed
> the calculation formula in the freelist_ptr(), which added a swab()
> call to mix bits a little more. When kernel is built with the
> "CONFIG_SLAB_FREELIST_HARDENED=y",the "kmem -s" option fails
with the
> following errors, if there is no such patch.
>
> crash> kmem -s
> CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
> 82166d00 144 0 0 0 4k fuse_request
> 82166e00 792 0 0 0 16k fuse_inode
> 87201e00 528 0 0 0 8k xfs_dqtrx
> 87201f00 496 0 0 0 8k xfs_dquot
> kmem: xfs_buf: slab: 37202e6e900 invalid freepointer: b844bab900001d70
> kmem: xfs_buf: slab: 3720250fd80 invalid freepointer: b8603f9400001370
> ...
Good catch! And the patch diff looks good to me.
Sorry, I completely misread the code.. Please ignore the comments below.
I will check again next week.
Thanks,
Kazu
But the freelist_ptr() function, which is patched, is called only when
the error message is NOT printed. So it seems like the patch does not
stop the message, right?
7:51 p.m.
On Sat, May 29, 2021 at 4:43 AM Kazuhito Hagio <kazuhito.hagio(a)gmail.com>
wrote:
> -----Original Message-----
> > Linux 5.7 and later kernels that contain kernel commit <1ad53d9fa3f6>
> > ("slub: improve bit diffusion for freelist ptr obfuscation") changed
> > the calculation formula in the freelist_ptr(), which added a swab()
> > call to mix bits a little more. When kernel is built with the
> > "CONFIG_SLAB_FREELIST_HARDENED=y",the "kmem -s" option
fails with the
> > following errors, if there is no such patch.
> >
> > crash> kmem -s
> > CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
> > 82166d00 144 0 0 0 4k
fuse_request
> > 82166e00 792 0 0 0 16k
fuse_inode
> > 87201e00 528 0 0 0 8k xfs_dqtrx
> > 87201f00 496 0 0 0 8k xfs_dquot
> > kmem: xfs_buf: slab: 37202e6e900 invalid freepointer: b844bab900001d70
> > kmem: xfs_buf: slab: 3720250fd80 invalid freepointer: b8603f9400001370
> > ...
>
> Good catch! And the patch diff looks good to me.
Sorry, I completely misread the code.. Please ignore the comments below.
I will check again next week.
No worry. Thanks for the review.
Thanks,
Kazu
>
> But the freelist_ptr() function, which is patched, is called only when
> the error message is NOT printed. So it seems like the patch does not
> stop the message, right?
No, the patch fixes the above errors. Upstream kernel has changed the
calculation
formula in the
freelist_ptr() as below(marked it with "^^^^"), which added the
"swab()"
operation, crash also needs
to follow up this change, otherwise crash will get the error of "invalid
freepointer".
diff --git a/mm/slub.c b/mm/slub.c
index fc911c222b11..bc949e3428c9 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -259,7 +259,7 @@ static inline void *freelist_ptr(const struct
kmem_cache *s, void *ptr,
* freepointer to be restored incorrectly.
*/
return (void *)((unsigned long)ptr ^ s->random ^
- (unsigned long)kasan_reset_tag((void *)ptr_addr));
+ *swab*((unsigned long)kasan_reset_tag((void
*)ptr_addr)));
* ^^^^^*
Before:
crash> kmem -s
CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
c0000000311a4200 152 0 0 0 64k fuse_request
c0000000311a8700 800 0 0 0 64k fuse_inode
c000000035df7600 528 0 0 0 64k xfs_dqtrx
c000000035df6600 496 0 0 0 64k xfs_dquot
kmem: xfs_buf: slab: c00c0000000b6500 invalid freepointer: 3808942d00004eb0
kmem: xfs_buf: slab: c00c00000116ba00 invalid freepointer: b83fe85a040027b0
...
After:
crash> kmem -s
CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
c0000000311a4200 152 0 0 0 64k fuse_request
c0000000311a8700 800 0 0 0 64k fuse_inode
c000000035df7600 528 0 0 0 64k xfs_dqtrx
c000000035df6600 496 0 0 0 64k xfs_dquot
c000000035dff500 360 7865 9010 53 64k xfs_buf
...
Thanks.
Lianbo
8:42 p.m.
Hi Lianbo,
No, the patch fixes the above errors. Upstream kernel has changed
the calculation formula in the
freelist_ptr() as below(marked it with "^^^^"), which added the
"swab()" operation, crash also needs
to follow up this change, otherwise crash will get the error of "invalid
freepointer".
Before:
crash> kmem -s
CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
c0000000311a4200 152 0 0 0 64k fuse_request
c0000000311a8700 800 0 0 0 64k fuse_inode
c000000035df7600 528 0 0 0 64k xfs_dqtrx
c000000035df6600 496 0 0 0 64k xfs_dquot
kmem: xfs_buf: slab: c00c0000000b6500 invalid freepointer: 3808942d00004eb0
kmem: xfs_buf: slab: c00c00000116ba00 invalid freepointer: b83fe85a040027b0
...
Yes, you are right, and thanks for the info.
I was confused because I could not reproduce the error message with a dozen
vmcores.
With some debugging, I think maybe the message itself does not occur on x86_64
due to this issue, because get_freepointer() returns a value having 1 in bit 1
on x86_64 and breaks the loop below.
@@ -19361,6 +19361,7 @@ get_freepointer(struct meminfo *si, void *object)
return BADADDR;
}
+ fprintf(fp, "DEBUG: vaddr: %lx ptr: %lx\n", vaddr,
freelist_ptr(si,nextfree,vaddr));
return (freelist_ptr(si, nextfree, vaddr));
}
crash> kmem -s
CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
...
DEBUG: vaddr: ffff927b0501b5b8 ptr: b8b501057b92ec47
DEBUG: vaddr: ffff927b05221bb8 ptr: b81b22057b92f8c7
DEBUG: vaddr: ffff927b05218e38 ptr: 388e21057b92f647
DEBUG: vaddr: ffff927b052269b8 ptr: b86922057b92f647
^bit1 = 1
ffff927b08452200 360 802 840 40 8k xfs_buf
DEBUG: vaddr: ffff927b07991900 ptr: 1999077b92f1f7
DEBUG: vaddr: ffff927b017ac130 ptr: 30c17a017b92f017
DEBUG: vaddr: ffff927b0496d9c8 ptr: c8d996047b92fcc7
DEBUG: vaddr: ffff927b01bce5e0 ptr: e0e5bc017b92f37f
ffff927b08452b00 200 50 480 24 4k xfs_bui_item
for (q = freelist; q; q = get_freepointer(si, (void *)q)) {
if (q & PAGE_MAPPING_ANON)
break; <<-- break here
However, the issue affects the calculation of allocated/free objects
also on x86_64:
-ffff927b08452200 360 802 840 40 8k xfs_buf
+ffff927b08452200 360 777 840 40 8k xfs_buf
^^^
> > Linux 5.7 and later kernels that contain kernel commit
<1ad53d9fa3f6>
> > ("slub: improve bit diffusion for freelist ptr obfuscation")
changed
> > the calculation formula in the freelist_ptr(), which added a swab()
> > call to mix bits a little more. When kernel is built with the
> > "CONFIG_SLAB_FREELIST_HARDENED=y",the "kmem -s" option
fails with the
> > following errors, if there is no such patch.
So, could I change the latter part of the commit message to the following?
When kernel is configured with the "CONFIG_SLAB_FREELIST_HARDENED=y",
Without the patch, the "kmem -s|-S" option displays wrong statistics and
state whether the slab objects are in use or free and can print the
following errors.
Thanks,
Kazu
9:39 p.m.
Hi, Kazu
Thank you for the comment.
On Mon, May 31, 2021 at 9:42 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
Hi Lianbo,
> No, the patch fixes the above errors. Upstream kernel has changed the
calculation formula in the
> freelist_ptr() as below(marked it with "^^^^"), which added the
"swab()"
operation, crash also needs
> to follow up this change, otherwise crash will get the error of "invalid
freepointer".
> Before:
> crash> kmem -s
> CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
> c0000000311a4200 152 0 0 0 64k
fuse_request
> c0000000311a8700 800 0 0 0 64k fuse_inode
> c000000035df7600 528 0 0 0 64k xfs_dqtrx
> c000000035df6600 496 0 0 0 64k xfs_dquot
> kmem: xfs_buf: slab: c00c0000000b6500 invalid freepointer:
3808942d00004eb0
> kmem: xfs_buf: slab: c00c00000116ba00 invalid freepointer:
b83fe85a040027b0
> ...
Yes, you are right, and thanks for the info.
I was confused because I could not reproduce the error message with a dozen
vmcores.
The above error messages are easily observed on ppc64le and s390 machines.
With some debugging, I think maybe the message itself does not occur on
x86_64
due to this issue, because get_freepointer() returns a value having 1 in
bit 1
on x86_64 and breaks the loop below.
@@ -19361,6 +19361,7 @@ get_freepointer(struct meminfo *si, void *object)
return BADADDR;
}
+ fprintf(fp, "DEBUG: vaddr: %lx ptr: %lx\n", vaddr,
freelist_ptr(si,nextfree,vaddr));
return (freelist_ptr(si, nextfree, vaddr));
}
crash> kmem -s
CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
...
DEBUG: vaddr: ffff927b0501b5b8 ptr: b8b501057b92ec47
DEBUG: vaddr: ffff927b05221bb8 ptr: b81b22057b92f8c7
DEBUG: vaddr: ffff927b05218e38 ptr: 388e21057b92f647
DEBUG: vaddr: ffff927b052269b8 ptr: b86922057b92f647
^bit1 = 1
ffff927b08452200 360 802 840 40 8k xfs_buf
DEBUG: vaddr: ffff927b07991900 ptr: 1999077b92f1f7
DEBUG: vaddr: ffff927b017ac130 ptr: 30c17a017b92f017
DEBUG: vaddr: ffff927b0496d9c8 ptr: c8d996047b92fcc7
DEBUG: vaddr: ffff927b01bce5e0 ptr: e0e5bc017b92f37f
ffff927b08452b00 200 50 480 24 4k xfs_bui_item
for (q = freelist; q; q = get_freepointer(si, (void *)q)) {
if (q & PAGE_MAPPING_ANON)
break; <<-- break here
However, the issue affects the calculation of allocated/free objects
also on x86_64:
-ffff927b08452200 360 802 840 40 8k xfs_buf
+ffff927b08452200 360 777 840 40 8k xfs_buf
^^^
> > > Linux 5.7 and later kernels that contain kernel commit
<1ad53d9fa3f6>
> > > ("slub: improve bit diffusion for freelist ptr
obfuscation")
changed
> > > the calculation formula in the freelist_ptr(), which added a
swab()
> > > call to mix bits a little more. When kernel is built with the
> > > "CONFIG_SLAB_FREELIST_HARDENED=y",the "kmem -s"
option fails
with the
> > > following errors, if there is no such patch.
So, could I change the latter part of the commit message to the following?
Yes, this also looks good.
Thanks.
Lianbo
When kernel is configured with the
"CONFIG_SLAB_FREELIST_HARDENED=y",
Without the patch, the "kmem -s|-S" option displays wrong statistics and
state whether the slab objects are in use or free and can print the
following errors.
Thanks,
Kazu
10:03 p.m.
-----Original Message-----
Hi, Kazu
Thank you for the comment.
On Mon, May 31, 2021 at 9:42 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com
<mailto:k-hagio-ab@nec.com>
> wrote:
Hi Lianbo,
> No, the patch fixes the above errors. Upstream kernel has changed the calculation
formula in the
> freelist_ptr() as below(marked it with "^^^^"), which added the
"swab()" operation, crash also
needs
> to follow up this change, otherwise crash will get the error of "invalid
freepointer".
> Before:
> crash> kmem -s
> CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
> c0000000311a4200 152 0 0 0 64k fuse_request
> c0000000311a8700 800 0 0 0 64k fuse_inode
> c000000035df7600 528 0 0 0 64k xfs_dqtrx
> c000000035df6600 496 0 0 0 64k xfs_dquot
> kmem: xfs_buf: slab: c00c0000000b6500 invalid freepointer: 3808942d00004eb0
> kmem: xfs_buf: slab: c00c00000116ba00 invalid freepointer: b83fe85a040027b0
> ...
Yes, you are right, and thanks for the info.
I was confused because I could not reproduce the error message with a dozen
vmcores.
The above error messages are easily observed on ppc64le and s390 machines.
With some debugging, I think maybe the message itself does not occur on x86_64
due to this issue, because get_freepointer() returns a value having 1 in bit 1
on x86_64 and breaks the loop below.
@@ -19361,6 +19361,7 @@ get_freepointer(struct meminfo *si, void *object)
return BADADDR;
}
+ fprintf(fp, "DEBUG: vaddr: %lx ptr: %lx\n", vaddr,
freelist_ptr(si,nextfree,vaddr));
return (freelist_ptr(si, nextfree, vaddr));
}
crash> kmem -s
CACHE OBJSIZE ALLOCATED TOTAL SLABS SSIZE NAME
...
DEBUG: vaddr: ffff927b0501b5b8 ptr: b8b501057b92ec47
DEBUG: vaddr: ffff927b05221bb8 ptr: b81b22057b92f8c7
DEBUG: vaddr: ffff927b05218e38 ptr: 388e21057b92f647
DEBUG: vaddr: ffff927b052269b8 ptr: b86922057b92f647
^bit1 = 1
ffff927b08452200 360 802 840 40 8k xfs_buf
DEBUG: vaddr: ffff927b07991900 ptr: 1999077b92f1f7
DEBUG: vaddr: ffff927b017ac130 ptr: 30c17a017b92f017
DEBUG: vaddr: ffff927b0496d9c8 ptr: c8d996047b92fcc7
DEBUG: vaddr: ffff927b01bce5e0 ptr: e0e5bc017b92f37f
ffff927b08452b00 200 50 480 24 4k xfs_bui_item
for (q = freelist; q; q = get_freepointer(si, (void *)q)) {
if (q & PAGE_MAPPING_ANON)
break; <<-- break here
However, the issue affects the calculation of allocated/free objects
also on x86_64:
-ffff927b08452200 360 802 840 40 8k xfs_buf
+ffff927b08452200 360 777 840 40 8k xfs_buf
^^^
> > > Linux 5.7 and later kernels that contain kernel commit
<1ad53d9fa3f6>
> > > ("slub: improve bit diffusion for freelist ptr
obfuscation") changed
> > > the calculation formula in the freelist_ptr(), which added a
swab()
> > > call to mix bits a little more. When kernel is built with the
> > > "CONFIG_SLAB_FREELIST_HARDENED=y",the "kmem -s"
option fails with the
> > > following errors, if there is no such patch.
So, could I change the latter part of the commit message to the following?
Yes, this also looks good.
Thanks, applied.
https://github.com/crash-utility/crash/commit/647a5c33e1c94054d7b63168cd6...
Kazu
1613
days inactive
1618
days old
devel@lists.crash-utility.osci.io
6 comments
4 participants
participants (4)
-
HAGIO KAZUHITO(萩尾 一仁) -
Kazuhito Hagio -
Lianbo Jiang -
lijiang