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

Failed BUILD_BUG in mm/mremap.c #1212

Closed
nathanchance opened this issue Dec 8, 2020 · 12 comments
Closed

Failed BUILD_BUG in mm/mremap.c #1212

nathanchance opened this issue Dec 8, 2020 · 12 comments
Assignees
Labels
[BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 5.11 This bug was fixed in Linux 5.11

Comments

@nathanchance
Copy link
Member

On next-20201208:

$ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- LLVM=1 O=out distclean allmodconfig vmlinux
...
ld.lld: error: undefined symbol: __compiletime_assert_398
>>> referenced by mremap.c
>>>               mremap.o:(get_extent) in archive mm/built-in.a
...

get_extent:

/*
 * Returns an extent of the corresponding size for the pgt_entry specified if
 * valid. Else returns a smaller extent bounded by the end of the source and
 * destination pgt_entry.
 */
static unsigned long get_extent(enum pgt_entry entry, unsigned long old_addr,
			unsigned long old_end, unsigned long new_addr)
{
	unsigned long next, extent, mask, size;

	switch (entry) {
	case HPAGE_PMD:
	case NORMAL_PMD:
		mask = PMD_MASK;
		size = PMD_SIZE;
		break;
	case NORMAL_PUD:
		mask = PUD_MASK;
		size = PUD_SIZE;
		break;
	default:
		BUILD_BUG();
		break;
	}

	next = (old_addr + size) & mask;
	/* even if next overflowed, extent below will be ok */
	extent = (next > old_end) ? old_end - old_addr : next - old_addr;
	next = (new_addr + size) & mask;
	if (extent > next - new_addr)
		extent = next - new_addr;
	return extent;
}

Bisect points out this is probably UBSAN related:

$ git bisect log
# bad: [a9e26cb5f2615cd638f911ea96d4fff5b4d93690] Add linux-next specific files for 20201208
# good: [cd796ed3345030aa1bb332fe5c793b3dddaf56e7] Merge tag 'trace-v5.10-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace
git bisect start 'a9e26cb5f2615cd638f911ea96d4fff5b4d93690' 'cd796ed3345030aa1bb332fe5c793b3dddaf56e7'
# good: [ad5711018462853bb0298d9c4caba35ee3e2a46e] Merge remote-tracking branch 'crypto/master'
git bisect good ad5711018462853bb0298d9c4caba35ee3e2a46e
# good: [6b415f03892954ad81976cd4025cf2afaa3d6ba7] Merge remote-tracking branch 'spi/for-next'
git bisect good 6b415f03892954ad81976cd4025cf2afaa3d6ba7
# good: [e4b95726aa03210eb2d74ba5fe1bd0d29f48c831] Merge remote-tracking branch 'staging/staging-next'
git bisect good e4b95726aa03210eb2d74ba5fe1bd0d29f48c831
# good: [0ba83260eb811bb3353d4437846e21a970e02a49] Merge remote-tracking branch 'userns/for-next'
git bisect good 0ba83260eb811bb3353d4437846e21a970e02a49
# good: [c0c70f8de1772385c98e4476e5c08bcfe5d4ea84] ath11k: make relay callbacks const
git bisect good c0c70f8de1772385c98e4476e5c08bcfe5d4ea84
# good: [bb87f5a0a0ef6f0c3553d3fec3249d3164dd533c] Merge remote-tracking branch 'auxdisplay/auxdisplay'
git bisect good bb87f5a0a0ef6f0c3553d3fec3249d3164dd533c
# bad: [1cfe18eb5ab53bf4086bd4afa7571a6eb412f615] epoll: replace gotos with a proper loop
git bisect bad 1cfe18eb5ab53bf4086bd4afa7571a6eb412f615
# good: [c26636d598da1890248ad157482e16c43b500ef9] scsi: block: fix for "scsi: block: Do not accept any requests while suspended"
git bisect good c26636d598da1890248ad157482e16c43b500ef9
# bad: [e0360dca9f06c66fe726724d571e883025329dc0] kmap: stupid hacks to make it compile
git bisect bad e0360dca9f06c66fe726724d571e883025329dc0
# bad: [f0350c9fad8a416a5797edb140e01bec21cc209e] ubsan: remove UBSAN_MISC in favor of individual options
git bisect bad f0350c9fad8a416a5797edb140e01bec21cc209e
# good: [995378db718490b00e56f0b749d3f1372c77c3c3] resource-fix-kernel-doc-markups-checkpatch-fixes
git bisect good 995378db718490b00e56f0b749d3f1372c77c3c3
# good: [2d60efe0a387aba744454ec2238b30285cf507b0] ubsan: disable object-size sanitizer under GCC
git bisect good 2d60efe0a387aba744454ec2238b30285cf507b0
# bad: [186c3e18dba3f035b73149f6356f8c1c439288f4] ubsan: enable for all*config builds
git bisect bad 186c3e18dba3f035b73149f6356f8c1c439288f4
# good: [a625b7b24fc318c6d731e941ce070a4d940ff143] ubsan: disable UBSAN_TRAP for all*config
git bisect good a625b7b24fc318c6d731e941ce070a4d940ff143
# first bad commit: [186c3e18dba3f035b73149f6356f8c1c439288f4] ubsan: enable for all*config builds

Sure enough, if I disable CONFIG_UBSAN_SANITIZE_ALL, the build passes just fine. I assume that this option prevents clang from eliminating the default statement? cc @kees

@nathanchance nathanchance added the [BUG] linux-next This is an issue only seen in linux-next label Dec 8, 2020
@nathanchance
Copy link
Member Author

This is now in mainline.

@nathanchance nathanchance added [BUG] linux A bug that should be fixed in the mainline kernel. and removed [BUG] linux-next This is an issue only seen in linux-next labels Dec 17, 2020
@nathanchance
Copy link
Member Author

Related to -fsanitize=unsigned-integer-overflow. cvise spits out:

enum { NORMAL_PMD } get_extent_old_addr;
get_extent_new_addr;
unsigned get_extent_next;
static get_extent(entry) {
  switch (entry) {
  case NORMAL_PMD:
    break;
  default:
    __compiletime_assert_1280();
  }
  get_extent_old_addr + --get_extent_next - get_extent_new_addr;
}
move_page_tables() { get_extent(); }

with this interestingness test.

@kees
Copy link

kees commented Dec 18, 2020

So this seems to be a UBSAN bug in Clang?

@kees
Copy link

kees commented Dec 18, 2020

I can't reproduce the failure.

builder@host:/srv/code$ cat mremap.i 
enum { NORMAL_PMD } get_extent_old_addr;
get_extent_new_addr;
unsigned get_extent_next;
static get_extent(entry) {
  switch (entry) {
  case NORMAL_PMD:
    break;
  default:
    __compiletime_assert_1280();
  }
  get_extent_old_addr + --get_extent_next - get_extent_new_addr;
}
move_page_tables() { get_extent(); }

builder@host:/srv/code$ clang-10 --version
Ubuntu clang version 10.0.1-6
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
builder@host:/srv/code$ rm mremap.o 
builder@host:/srv/code$ clang-10 -O2 -fsanitize=unsigned-integer-overflow -c -o mremap.o mremap.i 2>/dev/null
builder@host:/srv/code$ llvm-nm mremap.o 
                 U __ubsan_handle_add_overflow
                 U __ubsan_handle_sub_overflow
0000000000000004 C get_extent_new_addr
0000000000000004 C get_extent_next
0000000000000004 C get_extent_old_addr
0000000000000000 T move_page_tables

builder@host:/srv/code$ clang-12 --version
clang version 12.0.0 (https://github.com/llvm/llvm-project.git 0e94ba9d40d931fc1c647347302f39d24bd88d96)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /srv/llvm/x86/install/bin
builder@host:/srv/code$ rm mremap.o 
builder@host:/srv/code$ clang-12 -O2 -fsanitize=unsigned-integer-overflow -c -o mremap.o mremap.i 2>/dev/null
builder@host:/srv/code$ llvm-nm mremap.o 
                 U __ubsan_handle_add_overflow
                 U __ubsan_handle_sub_overflow
0000000000000004 B get_extent_new_addr
0000000000000008 B get_extent_next
0000000000000000 B get_extent_old_addr
0000000000000000 T move_page_tables

@nathanchance
Copy link
Member Author

Huh odd... I will try to double check myself after work. Can you at least reproduce the failure with ARCH=arm64 allmodconfig vmlinux?

@kees
Copy link

kees commented Dec 18, 2020

Yeah, I can't even build x86. Same issue. I feel like I'm doing something wrong... :P

@nathanchance
Copy link
Member Author

Hmmm cvise seems to have messed up... the final result fails the interestingness test :(

Oh well, used creduce to come up with:

enum { a } b;
c, d;
static e(g) {
  switch (g) {
  case a:
    break;
  default:
    __compiletime_assert_1280();
  }
  b + c - -b - d;
}
f() { e(); }

which reliably passes the interestingness test now.

@nickdesaulniers
Copy link
Member

Causing issues for Android: https://android-review.googlesource.com/c/kernel/common/+/1534178

@nathanchance
Copy link
Member Author

@nathanchance
Copy link
Member Author

Patch has been accepted: https://lore.kernel.org/mm-commits/20210204002030.FFOSUHLTU%25akpm@linux-foundation.org/

@nathanchance nathanchance added [PATCH] Accepted A submitted patch has been accepted upstream and removed [PATCH] Submitted A patch has been submitted for review labels Feb 5, 2021
@dileks
Copy link

dileks commented Feb 10, 2021

@nathanchance
Copy link
Member Author

@nathanchance nathanchance added [FIXED][LINUX] 5.11 This bug was fixed in Linux 5.11 and removed [PATCH] Accepted A submitted patch has been accepted upstream labels Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 5.11 This bug was fixed in Linux 5.11
Projects
None yet
Development

No branches or pull requests

5 participants