WARNING in notify_change

[syzbot]

Hello,

syzbot hit the following crash on upstream commit
3fd14cdcc05a682b03743683ce3a726898b20555 (Fri Apr 6 19:15:41 2018 +0000)
Merge tag 'mtd/for-4.17' of git://git.infradead.org/linux-mtd
syzbot dashboard link:
https://syzkaller.appspot.com/bug?extid=2b74da47f048a5046135

So far this crash happened 3 times on upstream.
C reproducer: https://syzkaller.appspot.com/x/repro.c?id=5084712116682752
syzkaller reproducer:
https://syzkaller.appspot.com/x/repro.syz?id=6394049002995712
Raw console output:
https://syzkaller.appspot.com/x/log.txt?id=4711531803574272
Kernel config:
https://syzkaller.appspot.com/x/.config?id=-5813481738265533882
compiler: gcc (GCC) 8.0.1 20180301 (experimental)

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+2b74da...@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for
details.
If you forward the report, please keep this part and the footer.

WARNING: CPU: 1 PID: 4470 at fs/attr.c:213 notify_change+0xd94/0x10c0
fs/attr.c:213
Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 4470 Comm: syzkaller570118 Not tainted 4.16.0+ #4
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1b9/0x294 lib/dump_stack.c:113
panic+0x22f/0x4de kernel/panic.c:183
__warn.cold.8+0x163/0x1a3 kernel/panic.c:547
report_bug+0x252/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
invalid_op+0x1b/0x40 arch/x86/entry/entry_64.S:991
RIP: 0010:notify_change+0xd94/0x10c0 fs/attr.c:213
RSP: 0018:ffff8801ad76f860 EFLAGS: 00010293
RAX: ffff8801d9f66680 RBX: ffff8801d32d4630 RCX: ffffffff81c36578
RDX: 0000000000000000 RSI: ffffffff81c371b4 RDI: 0000000000000007
RBP: ffff8801ad76f930 R08: ffff8801d9f66680 R09: ffffed003a65a8e2
R10: ffffed003a65a8e2 R11: ffff8801d32d4717 R12: 0000000000004200
R13: ffff8801ad76f970 R14: ffff8801d5260ce0 R15: 1ffff10035aedf18
__remove_privs fs/inode.c:1813 [inline]
file_remove_privs+0x2bf/0x530 fs/inode.c:1835
__generic_file_write_iter+0x169/0x5e0 mm/filemap.c:3209
blkdev_write_iter+0x233/0x420 fs/block_dev.c:1908
call_write_iter include/linux/fs.h:1782 [inline]
new_sync_write fs/read_write.c:474 [inline]
__vfs_write+0x5bc/0x880 fs/read_write.c:487
vfs_write+0x1f8/0x560 fs/read_write.c:549
ksys_write+0xf9/0x250 fs/read_write.c:598
SYSC_write fs/read_write.c:610 [inline]
SyS_write+0x24/0x30 fs/read_write.c:607
do_syscall_64+0x29e/0x9d0 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x445c59
RSP: 002b:00007f76203b7d38 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00000000006dac24 RCX: 0000000000445c59
RDX: 0000000000000050 RSI: 0000000020000400 RDI: 0000000000000008
RBP: 0000000000000005 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000293 R12: 00000000006dac20
R13: 0000000000000006 R14: 0030656c69662f2e R15: 0000000000000000
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.

syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is
merged
into any tree, please reply to this email with:
#syz fix: exact-commit-title
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line in the email body.

[Tetsuo Handa]

I haven't succeeded reproducing this bug using original C reproducer.
Instead, I'm observing XFS warning using modified reproducer shown below.

----------------------------------------
// autogenerated by syzkaller (http://github.com/google/syzkaller)

#include <fcntl.h>
#include <linux/futex.h>
#include <pthread.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>
#include <sys/xattr.h>

struct thread_t {
int created, running, call;
pthread_t th;
};

static struct thread_t threads[16];
static void execute_call(int call);
static int running;
static int collide;

static void* thr(void* arg)
{
struct thread_t* th = (struct thread_t*)arg;
for (;;) {
while (!__atomic_load_n(&th->running, __ATOMIC_ACQUIRE))
syscall(SYS_futex, &th->running, FUTEX_WAIT, 0, 0);
execute_call(th->call);
__atomic_fetch_sub(&running, 1, __ATOMIC_RELAXED);
__atomic_store_n(&th->running, 0, __ATOMIC_RELEASE);
syscall(SYS_futex, &th->running, FUTEX_WAKE);
}
return 0;
}

static void execute(int num_calls)
{
int call, thread;
running = 0;
for (call = 0; call < num_calls; call++) {
for (thread = 0; thread < sizeof(threads) / sizeof(threads[0]); thread++) {
struct thread_t* th = &threads[thread];
if (!th->created) {
th->created = 1;
pthread_attr_t attr;
pthread_attr_init(&attr);
pthread_attr_setstacksize(&attr, 128 << 10);
pthread_create(&th->th, &attr, thr, th);
}
if (!__atomic_load_n(&th->running, __ATOMIC_ACQUIRE)) {
th->call = call;
__atomic_fetch_add(&running, 1, __ATOMIC_RELAXED);
__atomic_store_n(&th->running, 1, __ATOMIC_RELEASE);
syscall(SYS_futex, &th->running, FUTEX_WAKE);
if (collide && call % 2)
break;
struct timespec ts;
ts.tv_sec = 0;
ts.tv_nsec = 20 * 1000 * 1000;
syscall(SYS_futex, &th->running, FUTEX_WAIT, 1, &ts);
if (running)
usleep((call == num_calls - 1) ? 10000 : 1000);
break;
}
}
}
}

static void execute_call(int call)
{
switch (call) {
case 0:
creat("file0", 0x20000080);
break;
case 1:
lsetxattr("file0", "security.capability",
"\x00\x00\x00\x02\x01\x00\x00\x00\x00\x00\x00\x01"
"\x04\x00\x00\x00\x00\x00\x00\x00", 20, 0);
break;
}
}

int main(int argc, char *argv[])
{
execute_call(0);
execute_call(1);
execute(2);
return 0;
}
----------------------------------------

When lsetxattr() is called in parallel (with CONFIG_XFS_WARN=y),
XFS emits below warning due to ATTR_KILL_PRIV not cleared yet.

----------------------------------------
[ 33.347151] XFS: Assertion failed: (iattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET| ATTR_MTIME_SET|ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0, file: fs/xfs/xfs_iops.c, line: 792
[ 33.353353] ------------[ cut here ]------------
[ 33.355376] WARNING: CPU: 0 PID: 5355 at fs/xfs/xfs_message.c:105 asswarn+0x2c/0x30 [xfs]
[ 33.358501] Modules linked in: ip6t_rpfilter ipt_REJECT nf_reject_ipv4 ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter vmw_balloon intel_powerclamp pcspkr vmw_vmci i2c_piix4 sg ppdev parport_pc parport ip_tables xfs libcrc32c sd_mod sr_mod cdrom ata_generic pata_acpi serio_raw vmwgfx drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops mptspi e1000 ttm scsi_transport_spi ahci mptscsih libahci drm ata_piix mptbase i2c_core libata
[ 33.385636] CPU: 0 PID: 5355 Comm: a.out Not tainted 4.9.0-rc1+ #55
[ 33.389051] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
[ 33.393952] ffffae56d0dafb78 ffffffff8f30c3ed 0000000000000000 0000000000000000
[ 33.398032] ffffae56d0dafbb8 ffffffff8f075431 00000069d0dafbd8 ffff9e5ca0ea1040
[ 33.401454] ffffae56d0dafcd8 ffff9e5ca0e9f778 0000000000000000 ffff9e5ca0ea1318
[ 33.404603] Call Trace:
[ 33.406183] [<ffffffff8f30c3ed>] dump_stack+0x8e/0xd1
[ 33.408541] [<ffffffff8f075431>] __warn+0xc1/0xe0
[ 33.410781] [<ffffffff8f075508>] warn_slowpath_null+0x18/0x20
[ 33.413358] [<ffffffffc06ac33c>] asswarn+0x2c/0x30 [xfs]
[ 33.415856] [<ffffffffc06a49b4>] xfs_setattr_size+0xc4/0x3a0 [xfs]
[ 33.418549] [<ffffffffc06a4cb7>] xfs_vn_setattr_size+0x27/0x30 [xfs]
[ 33.421409] [<ffffffffc06a4d18>] xfs_vn_setattr+0x58/0x80 [xfs]
[ 33.421413] [<ffffffff8f203aa3>] notify_change+0x313/0x430
[ 33.421415] [<ffffffff8f1dfc48>] do_truncate+0x58/0x90
[ 33.421417] [<ffffffff8f1f2e33>] path_openat+0xbc3/0xd20
[ 33.421419] [<ffffffff8f1f42eb>] do_filp_open+0x7b/0xd0
[ 33.421422] [<ffffffff8f6aa162>] ? _raw_spin_unlock+0x22/0x30
[ 33.421424] [<ffffffff8f204e13>] ? __alloc_fd+0xf3/0x200
[ 33.421425] [<ffffffff8f1e11ad>] do_sys_open+0x19d/0x230
[ 33.421426] [<ffffffff8f1e1299>] SyS_creat+0x19/0x20
[ 33.421429] [<ffffffff8f0036a7>] do_syscall_64+0x67/0x1f0
[ 33.421430] [<ffffffff8f6aa949>] entry_SYSCALL64_slow_path+0x25/0x25
[ 33.421481] ---[ end trace be3bb63a2a7fe0de ]---
----------------------------------------

Bisection reached commit 030b533c4fd4d2ec "fs: Avoid premature clearing of capabilities".
Since that commit changed whether ATTR_KILL_PRIV is removed from attr->ia_valid,
it is possible that ATTR_KILL_PRIV is delivered to xfs_setattr_size().
And making below change solves the XFS warning.

----------------------------------------
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 3b4be06..a67af5b 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -832,7 +832,7 @@
ASSERT(xfs_isilocked(ip, XFS_MMAPLOCK_EXCL));
ASSERT(S_ISREG(inode->i_mode));
ASSERT((iattr->ia_valid & (ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET|
- ATTR_MTIME_SET|ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0);
+ ATTR_MTIME_SET|ATTR_TIMES_SET)) == 0);

oldsize = inode->i_size;
newsize = iattr->ia_size;
----------------------------------------

But I have no idea why executing in parallel makes such difference.
Maybe parallel execution is something related to why syzbot is hitting
WARN_ON_ONCE(!inode_is_locked(inode)) in notify_change()...

[Khazhismel Kumykov]

I was able to reproduce this by setting security.capability xattr on a
blockdev file, then writing to it - when writing to the blockdev we
never lock the inode, so when we clear the capability we hit this
lockdep warning.

Is the issue here that we can set this xattr in the first place so we
have to clear it at all? Or should we really be locking the inode for
blockdevs after all? I'm not too familiar, but my gut says former

this reproducer is able to immediately crash machine running
linux-next-20190415:

#include <errno.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <unistd.h>
#include <attr/xattr.h>

char *disk = "/dev/loop0";

int main(void) {
int fd = open(disk, 0);
if (fd < 0)
printf("open: %d\n", errno);

system("dd if=/dev/zero of=a_file count=51200");
system("losetup /dev/loop0 a_file");

uint32_t value[5] = {
0x2000000,
7,
0x20d0,
6,
4
};

int res = fsetxattr(fd, "security.capability", &value,
sizeof(value), XATTR_CREATE);
if (res < 0)
printf ("xattr: %d\n", errno);

int fd2 = open(disk, O_RDWR);
write(fd2, "hello", 5);
return 0;
}

[Al Viro]

On Mon, Apr 15, 2019 at 04:20:17PM -0700, Khazhismel Kumykov wrote:
> I was able to reproduce this by setting security.capability xattr on a
> blockdev file, then writing to it - when writing to the blockdev we
> never lock the inode, so when we clear the capability we hit this
> lockdep warning.
>
> Is the issue here that we can set this xattr in the first place so we
> have to clear it at all? Or should we really be locking the inode for
> blockdevs after all? I'm not too familiar, but my gut says former

More interesting question is, WTF do we even touch that thing for
bdev? The thing is, mknod will cheerfully create any number of
different filesystem objects, all giving access to the same block
device. Which of them should have that xattr removed? It makes
no sense whatsoever; moreover, who *cares* about caps for block
device in the first place?

And if we did, what of another way to modify the block device?
You know, mount it read-write...

[Jan Kara]

Yes, Alexander Lochman has sent a patch to silence this warning back in
February [1] by just bailing out from file_remove_privs() for non-regular
files. But so far you've ignored that patch... Will you pick it up please?

Honza

[1] https://lore.kernel.org/lkml/cbdc8071-de76-bb0a...@tu-dortmund.de

--
Jan Kara <ja...@suse.com>
SUSE Labs, CR

[Eric Biggers]

#syz fix: Abort file_remove_privs() for non-reg. files