Hi, When testing the v6.1.69 kernel I bisected an issue to the below commit which was added in v6.1.68. When running the xfstests[1] on cifs I observe a null pointer dereference in cifs_flush_folio() because folio is null and dereferenced in size = folio_size(folio). [1] https://github.com/kdave/xfstests This can be reproduced using many of the xfstests but reproduces with generic/001 like below: $ ./check -cifs generic/001 FSTYP -- cifs PLATFORM -- Linux/x86_64 ip-172-31-36-150 6.1.69 #2 SMP PREEMPT_DYNAMIC Wed Jan 3 22:36:43 UTC 2024 MKFS_OPTIONS -- //172.31.34.66/scratch MOUNT_OPTIONS -- -ousername=ec2- user,password=abc123,noperm,mfsymlinks,cifsacl,actimeo=0 -o context=system_u:object_r:root_t:s0 //172.31.34.66/scratch /mnt/scratch generic/001 10s ... [ 244.135555] run fstests generic/001 at 2024-01-03 22:44:59 [ 244.637499] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ 244.638204] #PF: supervisor read access in kernel mode [ 244.638698] #PF: error_code(0x0000) - not-present page [ 244.639194] PGD 0 P4D 0 [ 244.639466] Oops: 0000 [#1] PREEMPT SMP NOPTI [ 244.698047] CPU: 11 PID: 4123 Comm: cp Not tainted 6.1.69 #2 [ 244.698598] Hardware name: Amazon EC2 m6i.4xlarge/, BIOS 1.0 10/16/2017 [ 244.699228] RIP: 0010:cifs_flush_folio+0x3f/0x100 [cifs] [ 244.699782] Code: d2 41 54 89 cc 31 c9 55 53 48 89 f3 48 c1 ee 0c 48 83 ec 08 48 8b 7f 30 e8 7d 2c a6 f8 48 3d 00 f0 ff ff 0f 87 a5 00 00 00 <48> 8b 10 48 89 c5 b8 00 10 00 00 f7 c2 00 00 01 00 74 07 0f b6 4d [ 244.799263] RSP: 0018:ffffc25441ffbd20 EFLAGS: 00010207 [ 244.888911] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 [ 244.898446] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9ed945eac000 [ 244.899136] RBP: 00000000000003a1 R08: 0000000000000001 R09: 0000000000000000 [ 244.997779] R10: 0000000000003e7f R11: 0000000000000000 R12: ffffc25441ffbd90 [ 244.998564] R13: ffffc25441ffbd88 R14: ffff9ed94af23850 R15: 0000000000000001 [ 244.999264] FS: 00007f111404d340(0000) GS:ffff9ee7fecc0000(0000) knlGS:0000000000000000 [ 245.097782] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 245.098345] CR2: 0000000000000000 CR3: 00000001211fc001 CR4: 00000000007706e0 [ 245.099122] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 245.099854] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 245.198200] PKRU: 55555554 [ 245.198478] Call Trace: [ 245.198735] <TASK> [ 245.198967] ? show_trace_log_lvl+0x1c4/0x2d2 [ 245.199399] ? show_trace_log_lvl+0x1c4/0x2d2 [ 245.199830] ? cifs_remap_file_range+0x15d/0x5e0 [cifs] [ 245.298029] ? __die_body.cold+0x8/0xd [ 245.298402] ? page_fault_oops+0xac/0x140 [ 245.298943] ? exc_page_fault+0x62/0x140 [ 245.299336] ? asm_exc_page_fault+0x22/0x30 [ 245.299751] ? cifs_flush_folio+0x3f/0x100 [cifs] [ 245.397858] ? cifs_precopy_set_eof+0x73/0x110 [cifs] [ 245.398383] cifs_remap_file_range+0x15d/0x5e0 [cifs] [ 245.398909] do_clone_file_range+0xe7/0x230 [ 245.399329] vfs_clone_file_range+0x37/0x140 [ 245.399861] ioctl_file_clone+0x45/0xa0 [ 245.497918] do_vfs_ioctl+0x79/0x890 [ 245.498328] __x64_sys_ioctl+0x69/0xc0 [ 245.498706] do_syscall_64+0x38/0x90 [ 245.499070] entry_SYSCALL_64_after_hwframe+0x64/0xce [ 245.499568] RIP: 0033:0x7f1113e3ec6b [ 245.499929] Code: 73 01 c3 48 8b 0d 9500 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 65 a1 1b 00 f7 d8 64 89 01 48 [ 245.599410] RSP: 002b:00007fff140ebb88 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 245.697930] RAX: ffffffffffffffda RBX: 00007fff140ec3b0 RCX: 00007f1113e3ec6b [ 245.698620] RDX: 0000000000000003 RSI: 0000000040049409 RDI: 0000000000000004 [ 245.699306] RBP: 0000000000000003 R08: 0000000000000001 R09: 0000000000000004 [ 245.797942] R10: 0000000000001000 R11: 0000000000000246 R12: 00007fff140ed616 [ 245.798789] R13: 00007fff140ebfa0 R14: 0000000000000000 R15: 0000000000000002 [ 245.799485] </TASK> [ 245.799720] Modules linked in: cmac nls_utf8 cifs cifs_arc4 cifs_md4 dns_lver mousedev sunrpc nls_ascii nls_cp437 vfat fat psmouse ghash_clmulni_intel atkbd libps2 vivaldi_fmap aesni_intel ena i8042 crypto_simd serio cryptd button drm sch_fq_codel fuse i2c_core configfs drm_panel_orientation_quirks loop backlight dmi_sysfs crc32_pclmul intel dm_mirror dm_region_hash dm_log dm_mod dax efivarfs [ 245.998457] CR2: 0000000000000000 [ 245.998800] ---[ end trace 0000000000000000 ]--- [ 246.087916] RIP: 0010:cifs_flush_folio+0x3f/0x100 [cifs] [ 246.088794] Code: d2 41 54 49 89 cc 31 c9 55 53 48 89 f3 48 c1 ee 0c 48 83 ec 08 48 8b 7f 30 e8 7d 2c a6 f8 48 3d 00 f0 ff ff 0f 87 a5 00 00 00 <48> 8b 10 48 89 c5 b8 00 10 00 00 f7 c2 00 00 01 00 74 07 0f b6 4d [ 246.099436] RSP: 0018:ffffc25441ffbd20 EFLAGS: 00010207 [ 246.189258] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 [ 246.198724] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9ed945eac000 [ 246.199411] RBP: 00000000000003a1 R08: 0000000000000001 R09: 0000000000000000 [ 246.298002] R10: 0000000000003e7f R11: 0000000000000000 R12: ffffc25441ffbd90 [ 246.298687] R13: ffffc25441ffbd88 R14: ffff9ed94af23850 R15: 0000000000000001 [ 246.299372] FS: 00007f111404d340(0000) GS:ffff9ee7fecc0000(0000) knlGS:0000000000000000 [ 246.398006] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 246.398569] CR2: 0000000000000000 CR3: 00000001211fc001 CR4: 00000000007706e0 [ 246.399254] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 246.399934] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 246.498414] PKRU: 55555554 [ 246.498698] Kernel panic - not syncing: Fatal exception [ 246.500042] Kernel Offset: 0x38000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff) [ 246.698094] Rebooting in 5 seconds.. Any ideas what is causing this and what the resolution is? This doesn't occur on the upstream/master kernel. Thanks, Suraj On Mon, 2023-12-11 at 13:57 +0100, gregkh@xxxxxxxxxxxxxxxxxxx wrote: > > This is a note to let you know that I've just added the patch titled > > cifs: Fix flushing, invalidation and file size with > copy_file_range() > > to the 6.1-stable tree which can be found at: > > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > The filename of the patch is: > cifs-fix-flushing-invalidation-and-file-size-with- > copy_file_range.patch > and it can be found in the queue-6.1 subdirectory. > > If you, or anyone else, feels it should not be added to the stable > tree, > please let <stable@xxxxxxxxxxxxxxx> know about it. > > > > From 7b2404a886f8b91250c31855d287e632123e1746 Mon Sep 17 00:00:00 > > 2001 > From: David Howells <dhowells@xxxxxxxxxx> > Date: Fri, 1 Dec 2023 00:22:00 +0000 > Subject: cifs: Fix flushing, invalidation and file size with > copy_file_range() > > From: David Howells <dhowells@xxxxxxxxxx> > > commit 7b2404a886f8b91250c31855d287e632123e1746 upstream. > > Fix a number of issues in the cifs filesystem implementation of the > copy_file_range() syscall in cifs_file_copychunk_range(). > > Firstly, the invalidation of the destination range is handled > incorrectly: > We shouldn't just invalidate the whole file as dirty data in the file > may > get lost and we can't just call truncate_inode_pages_range() to > invalidate > the destination range as that will erase parts of a partial folio at > each > end whilst invalidating and discarding all the folios in the middle. > We > need to force all the folios covering the range to be reloaded, but > we > mustn't lose dirty data in them that's not in the destination range. > > Further, we shouldn't simply round out the range to PAGE_SIZE at each > end > as cifs should move to support multipage folios. > > Secondly, there's an issue whereby a write may have extended the file > locally, but not have been written back yet. This can leaves the > local > idea of the EOF at a later point than the server's EOF. If a copy > request > is issued, this will fail on the server with STATUS_INVALID_VIEW_SIZE > (which gets translated to -EIO locally) if the copy source extends > past the > server's EOF. > > Fix this by: > > (0) Flush the source region (already done). The flush does nothing > and > the EOF isn't moved if the source region has no dirty data. > > (1) Move the EOF to the end of the source region if it isn't already > at > least at this point. If we can't do this, for instance if the > server > doesn't support it, just flush the entire source file. > > (2) Find the folio (if present) at each end of the range, flushing > it and > increasing the region-to-be-invalidated to cover those in their > entirety. > > (3) Fully discard all the folios covering the range as we want them > to be > reloaded. > > (4) Then perform the copy. > > Thirdly, set i_size after doing the copychunk_range operation as this > value > may be used by various things internally. stat() hides the issue > because > setting ->time to 0 causes cifs_getatr() to revalidate the > attributes. > > These were causing the generic/075 xfstest to fail. > > Fixes: 620d8745b35d ("Introduce cifs_copy_file_range()") > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: David Howells <dhowells@xxxxxxxxxx> > cc: Paulo Alcantara <pc@xxxxxxxxxxxxx> > cc: Shyam Prasad N <nspmangalore@xxxxxxxxx> > cc: Rohith Surabattula <rohiths.msft@xxxxxxxxx> > cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> > cc: Jeff Layton <jlayton@xxxxxxxxxx> > cc: linux-cifs@xxxxxxxxxxxxxxx > cc: linux-mm@xxxxxxxxx > Signed-off-by: David Howells <dhowells@xxxxxxxxxx> > Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > --- > fs/smb/client/cifsfs.c | 102 > +++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 99 insertions(+), 3 deletions(-) > > --- a/fs/smb/client/cifsfs.c > +++ b/fs/smb/client/cifsfs.c > @@ -1191,6 +1191,72 @@ const struct inode_operations cifs_symli > .listxattr = cifs_listxattr, > }; > > +/* > + * Advance the EOF marker to after the source range. > + */ > +static int cifs_precopy_set_eof(struct inode *src_inode, struct > cifsInodeInfo *src_cifsi, > + struct cifs_tcon *src_tcon, > + unsigned int xid, loff_t src_end) > +{ > + struct cifsFileInfo *writeable_srcfile; > + int rc = -EINVAL; > + > + writeable_srcfile = find_writable_file(src_cifsi, > FIND_WR_FSUID_ONLY); > + if (writeable_srcfile) { > + if (src_tcon->ses->server->ops->set_file_size) > + rc = src_tcon->ses->server->ops- > >set_file_size( > + xid, src_tcon, writeable_srcfile, > + src_inode->i_size, true /* no need to > set sparse */); > + else > + rc = -ENOSYS; > + cifsFileInfo_put(writeable_srcfile); > + cifs_dbg(FYI, "SetFSize for copychunk rc = %d\n", > rc); > + } > + > + if (rc < 0) > + goto set_failed; > + > + netfs_resize_file(&src_cifsi->netfs, src_end); > + fscache_resize_cookie(cifs_inode_cookie(src_inode), src_end); > + return 0; > + > +set_failed: > + return filemap_write_and_wait(src_inode->i_mapping); > +} > + > +/* > + * Flush out either the folio that overlaps the beginning of a range > in which > + * pos resides or the folio that overlaps the end of a range unless > that folio > + * is entirely within the range we're going to invalidate. We > extend the flush > + * bounds to encompass the folio. > + */ > +static int cifs_flush_folio(struct inode *inode, loff_t pos, loff_t > *_fstart, loff_t *_fend, > + bool first) > +{ > + struct folio *folio; > + unsigned long long fpos, fend; > + pgoff_t index = pos / PAGE_SIZE; > + size_t size; > + int rc = 0; > + > + folio = filemap_get_folio(inode->i_mapping, index); > + if (IS_ERR(folio)) > + return 0; > + > + size = folio_size(folio); > + fpos = folio_pos(folio); > + fend = fpos + size - 1; > + *_fstart = min_t(unsigned long long, *_fstart, fpos); > + *_fend = max_t(unsigned long long, *_fend, fend); > + if ((first && pos == fpos) || (!first && pos == fend)) > + goto out; > + > + rc = filemap_write_and_wait_range(inode->i_mapping, fpos, > fend); > +out: > + folio_put(folio); > + return rc; > +} > + > static loff_t cifs_remap_file_range(struct file *src_file, loff_t > off, > struct file *dst_file, loff_t destoff, loff_t len, > unsigned int remap_flags) > @@ -1260,10 +1326,12 @@ ssize_t cifs_file_copychunk_range(unsign > { > struct inode *src_inode = file_inode(src_file); > struct inode *target_inode = file_inode(dst_file); > + struct cifsInodeInfo *src_cifsi = CIFS_I(src_inode); > struct cifsFileInfo *smb_file_src; > struct cifsFileInfo *smb_file_target; > struct cifs_tcon *src_tcon; > struct cifs_tcon *target_tcon; > + unsigned long long destend, fstart, fend; > ssize_t rc; > > cifs_dbg(FYI, "copychunk range\n"); > @@ -1303,13 +1371,41 @@ ssize_t cifs_file_copychunk_range(unsign > if (rc) > goto unlock; > > - /* should we flush first and last page first */ > - truncate_inode_pages(&target_inode->i_data, 0); > + /* The server-side copy will fail if the source crosses the > EOF marker. > + * Advance the EOF marker after the flush above to the end of > the range > + * if it's short of that. > + */ > + if (src_cifsi->server_eof < off + len) { > + rc = cifs_precopy_set_eof(src_inode, src_cifsi, > src_tcon, xid, off + len); > + if (rc < 0) > + goto unlock; > + } > + > + destend = destoff + len - 1; > + > + /* Flush the folios at either end of the destination range to > prevent > + * accidental loss of dirty data outside of the range. > + */ > + fstart = destoff; > + fend = destend; > + > + rc = cifs_flush_folio(target_inode, destoff, &fstart, &fend, > true); > + if (rc) > + goto unlock; > + rc = cifs_flush_folio(target_inode, destend, &fstart, &fend, > false); > + if (rc) > + goto unlock; > + > + /* Discard all the folios that overlap the destination > region. */ > + truncate_inode_pages_range(&target_inode->i_data, fstart, > fend); > > rc = file_modified(dst_file); > - if (!rc) > + if (!rc) { > rc = target_tcon->ses->server->ops- > >copychunk_range(xid, > smb_file_src, smb_file_target, off, len, > destoff); > + if (rc > 0 && destoff + rc > > i_size_read(target_inode)) > + truncate_setsize(target_inode, destoff + rc); > + } > > file_accessed(src_file); > > > > Patches currently in stable-queue which might be from > dhowells@xxxxxxxxxx are > > queue-6.1/cifs-fix-flushing-invalidation-and-file-size-with- > copy_file_range.patch > queue-6.1/cifs-fix-flushing-invalidation-and-file-size-with- > ficlone.patch > queue-6.1/cifs-fix-non-availability-of-dedup-breaking-generic- > 304.patch >
