All of lore.kernel.org
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to" failed to apply to 4.19-stable tree
@ 2019-10-14 16:18 gregkh
  2019-10-14 16:38 ` David Wysochanski
  0 siblings, 1 reply; 6+ messages in thread
From: gregkh @ 2019-10-14 16:18 UTC (permalink / raw)
  To: dwysocha, lsahlber, stable, stfrench; +Cc: stable


The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From cb248819d209d113e45fed459773991518e8e80b Mon Sep 17 00:00:00 2001
From: Dave Wysochanski <dwysocha@redhat.com>
Date: Thu, 3 Oct 2019 15:16:27 +1000
Subject: [PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to
 avoid a panic

Commit 487317c99477 ("cifs: add spinlock for the openFileList to
cifsInodeInfo") added cifsInodeInfo->open_file_lock spin_lock to protect
the openFileList, but missed a few places where cifs_inode->openFileList
was enumerated.  Change these remaining tcon->open_file_lock to
cifsInodeInfo->open_file_lock to avoid panic in is_size_safe_to_change.

[17313.245641] RIP: 0010:is_size_safe_to_change+0x57/0xb0 [cifs]
[17313.245645] Code: 68 40 48 89 ef e8 19 67 b7 f1 48 8b 43 40 48 8d 4b 40 48 8d 50 f0 48 39 c1 75 0f eb 47 48 8b 42 10 48 8d 50 f0 48 39 c1 74 3a <8b> 80 88 00 00 00 83 c0 01 a8 02 74 e6 48 89 ef c6 07 00 0f 1f 40
[17313.245649] RSP: 0018:ffff94ae1baefa30 EFLAGS: 00010202
[17313.245654] RAX: dead000000000100 RBX: ffff88dc72243300 RCX: ffff88dc72243340
[17313.245657] RDX: dead0000000000f0 RSI: 00000000098f7940 RDI: ffff88dd3102f040
[17313.245659] RBP: ffff88dd3102f040 R08: 0000000000000000 R09: ffff94ae1baefc40
[17313.245661] R10: ffffcdc8bb1c4e80 R11: ffffcdc8b50adb08 R12: 00000000098f7940
[17313.245663] R13: ffff88dc72243300 R14: ffff88dbc8f19600 R15: ffff88dc72243428
[17313.245667] FS:  00007fb145485700(0000) GS:ffff88dd3e000000(0000) knlGS:0000000000000000
[17313.245670] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[17313.245672] CR2: 0000026bb46c6000 CR3: 0000004edb110003 CR4: 00000000007606e0
[17313.245753] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[17313.245756] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[17313.245759] PKRU: 55555554
[17313.245761] Call Trace:
[17313.245803]  cifs_fattr_to_inode+0x16b/0x580 [cifs]
[17313.245838]  cifs_get_inode_info+0x35c/0xa60 [cifs]
[17313.245852]  ? kmem_cache_alloc_trace+0x151/0x1d0
[17313.245885]  cifs_open+0x38f/0x990 [cifs]
[17313.245921]  ? cifs_revalidate_dentry_attr+0x3e/0x350 [cifs]
[17313.245953]  ? cifsFileInfo_get+0x30/0x30 [cifs]
[17313.245960]  ? do_dentry_open+0x132/0x330
[17313.245963]  do_dentry_open+0x132/0x330
[17313.245969]  path_openat+0x573/0x14d0
[17313.245974]  do_filp_open+0x93/0x100
[17313.245979]  ? __check_object_size+0xa3/0x181
[17313.245986]  ? audit_alloc_name+0x7e/0xd0
[17313.245992]  do_sys_open+0x184/0x220
[17313.245999]  do_syscall_64+0x5b/0x1b0

Fixes: 487317c99477 ("cifs: add spinlock for the openFileList to cifsInodeInfo")

CC: Stable <stable@vger.kernel.org>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 4b95700c507c..3758237bf951 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1840,13 +1840,12 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
 {
 	struct cifsFileInfo *open_file = NULL;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb);
-	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
 
 	/* only filter by fsuid on multiuser mounts */
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
 		fsuid_only = false;
 
-	spin_lock(&tcon->open_file_lock);
+	spin_lock(&cifs_inode->open_file_lock);
 	/* we could simply get the first_list_entry since write-only entries
 	   are always at the end of the list but since the first entry might
 	   have a close pending, we go through the whole list */
@@ -1858,7 +1857,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
 				/* found a good file */
 				/* lock it so it will not be closed on us */
 				cifsFileInfo_get(open_file);
-				spin_unlock(&tcon->open_file_lock);
+				spin_unlock(&cifs_inode->open_file_lock);
 				return open_file;
 			} /* else might as well continue, and look for
 			     another, or simply have the caller reopen it
@@ -1866,7 +1865,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
 		} else /* write only file */
 			break; /* write only files are last so must be done */
 	}
-	spin_unlock(&tcon->open_file_lock);
+	spin_unlock(&cifs_inode->open_file_lock);
 	return NULL;
 }
 
@@ -1877,7 +1876,6 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
 {
 	struct cifsFileInfo *open_file, *inv_file = NULL;
 	struct cifs_sb_info *cifs_sb;
-	struct cifs_tcon *tcon;
 	bool any_available = false;
 	int rc = -EBADF;
 	unsigned int refind = 0;
@@ -1897,16 +1895,15 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
 	}
 
 	cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb);
-	tcon = cifs_sb_master_tcon(cifs_sb);
 
 	/* only filter by fsuid on multiuser mounts */
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
 		fsuid_only = false;
 
-	spin_lock(&tcon->open_file_lock);
+	spin_lock(&cifs_inode->open_file_lock);
 refind_writable:
 	if (refind > MAX_REOPEN_ATT) {
-		spin_unlock(&tcon->open_file_lock);
+		spin_unlock(&cifs_inode->open_file_lock);
 		return rc;
 	}
 	list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
@@ -1918,7 +1915,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
 			if (!open_file->invalidHandle) {
 				/* found a good writable file */
 				cifsFileInfo_get(open_file);
-				spin_unlock(&tcon->open_file_lock);
+				spin_unlock(&cifs_inode->open_file_lock);
 				*ret_file = open_file;
 				return 0;
 			} else {
@@ -1938,7 +1935,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
 		cifsFileInfo_get(inv_file);
 	}
 
-	spin_unlock(&tcon->open_file_lock);
+	spin_unlock(&cifs_inode->open_file_lock);
 
 	if (inv_file) {
 		rc = cifs_reopen_file(inv_file, false);
@@ -1953,7 +1950,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
 		cifsFileInfo_put(inv_file);
 		++refind;
 		inv_file = NULL;
-		spin_lock(&tcon->open_file_lock);
+		spin_lock(&cifs_inode->open_file_lock);
 		goto refind_writable;
 	}
 
@@ -4461,17 +4458,15 @@ static int cifs_readpage(struct file *file, struct page *page)
 static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
 {
 	struct cifsFileInfo *open_file;
-	struct cifs_tcon *tcon =
-		cifs_sb_master_tcon(CIFS_SB(cifs_inode->vfs_inode.i_sb));
 
-	spin_lock(&tcon->open_file_lock);
+	spin_lock(&cifs_inode->open_file_lock);
 	list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
 		if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
-			spin_unlock(&tcon->open_file_lock);
+			spin_unlock(&cifs_inode->open_file_lock);
 			return 1;
 		}
 	}
-	spin_unlock(&tcon->open_file_lock);
+	spin_unlock(&cifs_inode->open_file_lock);
 	return 0;
 }
 


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: FAILED: patch "[PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to" failed to apply to 4.19-stable tree
  2019-10-14 16:18 FAILED: patch "[PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to" failed to apply to 4.19-stable tree gregkh
@ 2019-10-14 16:38 ` David Wysochanski
  2019-10-14 20:38   ` David Wysochanski
  2019-10-15  3:29   ` Sasha Levin
  0 siblings, 2 replies; 6+ messages in thread
From: David Wysochanski @ 2019-10-14 16:38 UTC (permalink / raw)
  To: gregkh; +Cc: Ronnie Sahlberg, stable, stfrench

Unless there is objections, let me try to fix this up.
Thanks.

On Mon, Oct 14, 2019 at 12:18 PM <gregkh@linuxfoundation.org> wrote:
>
>
> The patch below does not apply to the 4.19-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
>
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> From cb248819d209d113e45fed459773991518e8e80b Mon Sep 17 00:00:00 2001
> From: Dave Wysochanski <dwysocha@redhat.com>
> Date: Thu, 3 Oct 2019 15:16:27 +1000
> Subject: [PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to
>  avoid a panic
>
> Commit 487317c99477 ("cifs: add spinlock for the openFileList to
> cifsInodeInfo") added cifsInodeInfo->open_file_lock spin_lock to protect
> the openFileList, but missed a few places where cifs_inode->openFileList
> was enumerated.  Change these remaining tcon->open_file_lock to
> cifsInodeInfo->open_file_lock to avoid panic in is_size_safe_to_change.
>
> [17313.245641] RIP: 0010:is_size_safe_to_change+0x57/0xb0 [cifs]
> [17313.245645] Code: 68 40 48 89 ef e8 19 67 b7 f1 48 8b 43 40 48 8d 4b 40 48 8d 50 f0 48 39 c1 75 0f eb 47 48 8b 42 10 48 8d 50 f0 48 39 c1 74 3a <8b> 80 88 00 00 00 83 c0 01 a8 02 74 e6 48 89 ef c6 07 00 0f 1f 40
> [17313.245649] RSP: 0018:ffff94ae1baefa30 EFLAGS: 00010202
> [17313.245654] RAX: dead000000000100 RBX: ffff88dc72243300 RCX: ffff88dc72243340
> [17313.245657] RDX: dead0000000000f0 RSI: 00000000098f7940 RDI: ffff88dd3102f040
> [17313.245659] RBP: ffff88dd3102f040 R08: 0000000000000000 R09: ffff94ae1baefc40
> [17313.245661] R10: ffffcdc8bb1c4e80 R11: ffffcdc8b50adb08 R12: 00000000098f7940
> [17313.245663] R13: ffff88dc72243300 R14: ffff88dbc8f19600 R15: ffff88dc72243428
> [17313.245667] FS:  00007fb145485700(0000) GS:ffff88dd3e000000(0000) knlGS:0000000000000000
> [17313.245670] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [17313.245672] CR2: 0000026bb46c6000 CR3: 0000004edb110003 CR4: 00000000007606e0
> [17313.245753] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [17313.245756] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [17313.245759] PKRU: 55555554
> [17313.245761] Call Trace:
> [17313.245803]  cifs_fattr_to_inode+0x16b/0x580 [cifs]
> [17313.245838]  cifs_get_inode_info+0x35c/0xa60 [cifs]
> [17313.245852]  ? kmem_cache_alloc_trace+0x151/0x1d0
> [17313.245885]  cifs_open+0x38f/0x990 [cifs]
> [17313.245921]  ? cifs_revalidate_dentry_attr+0x3e/0x350 [cifs]
> [17313.245953]  ? cifsFileInfo_get+0x30/0x30 [cifs]
> [17313.245960]  ? do_dentry_open+0x132/0x330
> [17313.245963]  do_dentry_open+0x132/0x330
> [17313.245969]  path_openat+0x573/0x14d0
> [17313.245974]  do_filp_open+0x93/0x100
> [17313.245979]  ? __check_object_size+0xa3/0x181
> [17313.245986]  ? audit_alloc_name+0x7e/0xd0
> [17313.245992]  do_sys_open+0x184/0x220
> [17313.245999]  do_syscall_64+0x5b/0x1b0
>
> Fixes: 487317c99477 ("cifs: add spinlock for the openFileList to cifsInodeInfo")
>
> CC: Stable <stable@vger.kernel.org>
> Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
> Signed-off-by: Steve French <stfrench@microsoft.com>
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 4b95700c507c..3758237bf951 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1840,13 +1840,12 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
>  {
>         struct cifsFileInfo *open_file = NULL;
>         struct cifs_sb_info *cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb);
> -       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
>
>         /* only filter by fsuid on multiuser mounts */
>         if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
>                 fsuid_only = false;
>
> -       spin_lock(&tcon->open_file_lock);
> +       spin_lock(&cifs_inode->open_file_lock);
>         /* we could simply get the first_list_entry since write-only entries
>            are always at the end of the list but since the first entry might
>            have a close pending, we go through the whole list */
> @@ -1858,7 +1857,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
>                                 /* found a good file */
>                                 /* lock it so it will not be closed on us */
>                                 cifsFileInfo_get(open_file);
> -                               spin_unlock(&tcon->open_file_lock);
> +                               spin_unlock(&cifs_inode->open_file_lock);
>                                 return open_file;
>                         } /* else might as well continue, and look for
>                              another, or simply have the caller reopen it
> @@ -1866,7 +1865,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
>                 } else /* write only file */
>                         break; /* write only files are last so must be done */
>         }
> -       spin_unlock(&tcon->open_file_lock);
> +       spin_unlock(&cifs_inode->open_file_lock);
>         return NULL;
>  }
>
> @@ -1877,7 +1876,6 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
>  {
>         struct cifsFileInfo *open_file, *inv_file = NULL;
>         struct cifs_sb_info *cifs_sb;
> -       struct cifs_tcon *tcon;
>         bool any_available = false;
>         int rc = -EBADF;
>         unsigned int refind = 0;
> @@ -1897,16 +1895,15 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
>         }
>
>         cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb);
> -       tcon = cifs_sb_master_tcon(cifs_sb);
>
>         /* only filter by fsuid on multiuser mounts */
>         if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
>                 fsuid_only = false;
>
> -       spin_lock(&tcon->open_file_lock);
> +       spin_lock(&cifs_inode->open_file_lock);
>  refind_writable:
>         if (refind > MAX_REOPEN_ATT) {
> -               spin_unlock(&tcon->open_file_lock);
> +               spin_unlock(&cifs_inode->open_file_lock);
>                 return rc;
>         }
>         list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
> @@ -1918,7 +1915,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
>                         if (!open_file->invalidHandle) {
>                                 /* found a good writable file */
>                                 cifsFileInfo_get(open_file);
> -                               spin_unlock(&tcon->open_file_lock);
> +                               spin_unlock(&cifs_inode->open_file_lock);
>                                 *ret_file = open_file;
>                                 return 0;
>                         } else {
> @@ -1938,7 +1935,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
>                 cifsFileInfo_get(inv_file);
>         }
>
> -       spin_unlock(&tcon->open_file_lock);
> +       spin_unlock(&cifs_inode->open_file_lock);
>
>         if (inv_file) {
>                 rc = cifs_reopen_file(inv_file, false);
> @@ -1953,7 +1950,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
>                 cifsFileInfo_put(inv_file);
>                 ++refind;
>                 inv_file = NULL;
> -               spin_lock(&tcon->open_file_lock);
> +               spin_lock(&cifs_inode->open_file_lock);
>                 goto refind_writable;
>         }
>
> @@ -4461,17 +4458,15 @@ static int cifs_readpage(struct file *file, struct page *page)
>  static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
>  {
>         struct cifsFileInfo *open_file;
> -       struct cifs_tcon *tcon =
> -               cifs_sb_master_tcon(CIFS_SB(cifs_inode->vfs_inode.i_sb));
>
> -       spin_lock(&tcon->open_file_lock);
> +       spin_lock(&cifs_inode->open_file_lock);
>         list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
>                 if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
> -                       spin_unlock(&tcon->open_file_lock);
> +                       spin_unlock(&cifs_inode->open_file_lock);
>                         return 1;
>                 }
>         }
> -       spin_unlock(&tcon->open_file_lock);
> +       spin_unlock(&cifs_inode->open_file_lock);
>         return 0;
>  }
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: FAILED: patch "[PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to" failed to apply to 4.19-stable tree
  2019-10-14 16:38 ` David Wysochanski
@ 2019-10-14 20:38   ` David Wysochanski
  2019-10-15  5:24     ` Sasha Levin
  2019-10-15  3:29   ` Sasha Levin
  1 sibling, 1 reply; 6+ messages in thread
From: David Wysochanski @ 2019-10-14 20:38 UTC (permalink / raw)
  To: gregkh; +Cc: Ronnie Sahlberg, stable, stfrench

Original patch applies cleanly to 5.1-5.3. Prior to that (4.19-5.0),
needs minor fixups.
Do I need to re-send the original and the fixup with different kernel ranges?

On Mon, Oct 14, 2019 at 12:38 PM David Wysochanski <dwysocha@redhat.com> wrote:
>
> Unless there is objections, let me try to fix this up.
> Thanks.
>
> On Mon, Oct 14, 2019 at 12:18 PM <gregkh@linuxfoundation.org> wrote:
> >
> >
> > The patch below does not apply to the 4.19-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> >
> > thanks,
> >
> > greg k-h
> >
> > ------------------ original commit in Linus's tree ------------------
> >
> > From cb248819d209d113e45fed459773991518e8e80b Mon Sep 17 00:00:00 2001
> > From: Dave Wysochanski <dwysocha@redhat.com>
> > Date: Thu, 3 Oct 2019 15:16:27 +1000
> > Subject: [PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to
> >  avoid a panic
> >
> > Commit 487317c99477 ("cifs: add spinlock for the openFileList to
> > cifsInodeInfo") added cifsInodeInfo->open_file_lock spin_lock to protect
> > the openFileList, but missed a few places where cifs_inode->openFileList
> > was enumerated.  Change these remaining tcon->open_file_lock to
> > cifsInodeInfo->open_file_lock to avoid panic in is_size_safe_to_change.
> >
> > [17313.245641] RIP: 0010:is_size_safe_to_change+0x57/0xb0 [cifs]
> > [17313.245645] Code: 68 40 48 89 ef e8 19 67 b7 f1 48 8b 43 40 48 8d 4b 40 48 8d 50 f0 48 39 c1 75 0f eb 47 48 8b 42 10 48 8d 50 f0 48 39 c1 74 3a <8b> 80 88 00 00 00 83 c0 01 a8 02 74 e6 48 89 ef c6 07 00 0f 1f 40
> > [17313.245649] RSP: 0018:ffff94ae1baefa30 EFLAGS: 00010202
> > [17313.245654] RAX: dead000000000100 RBX: ffff88dc72243300 RCX: ffff88dc72243340
> > [17313.245657] RDX: dead0000000000f0 RSI: 00000000098f7940 RDI: ffff88dd3102f040
> > [17313.245659] RBP: ffff88dd3102f040 R08: 0000000000000000 R09: ffff94ae1baefc40
> > [17313.245661] R10: ffffcdc8bb1c4e80 R11: ffffcdc8b50adb08 R12: 00000000098f7940
> > [17313.245663] R13: ffff88dc72243300 R14: ffff88dbc8f19600 R15: ffff88dc72243428
> > [17313.245667] FS:  00007fb145485700(0000) GS:ffff88dd3e000000(0000) knlGS:0000000000000000
> > [17313.245670] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [17313.245672] CR2: 0000026bb46c6000 CR3: 0000004edb110003 CR4: 00000000007606e0
> > [17313.245753] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [17313.245756] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [17313.245759] PKRU: 55555554
> > [17313.245761] Call Trace:
> > [17313.245803]  cifs_fattr_to_inode+0x16b/0x580 [cifs]
> > [17313.245838]  cifs_get_inode_info+0x35c/0xa60 [cifs]
> > [17313.245852]  ? kmem_cache_alloc_trace+0x151/0x1d0
> > [17313.245885]  cifs_open+0x38f/0x990 [cifs]
> > [17313.245921]  ? cifs_revalidate_dentry_attr+0x3e/0x350 [cifs]
> > [17313.245953]  ? cifsFileInfo_get+0x30/0x30 [cifs]
> > [17313.245960]  ? do_dentry_open+0x132/0x330
> > [17313.245963]  do_dentry_open+0x132/0x330
> > [17313.245969]  path_openat+0x573/0x14d0
> > [17313.245974]  do_filp_open+0x93/0x100
> > [17313.245979]  ? __check_object_size+0xa3/0x181
> > [17313.245986]  ? audit_alloc_name+0x7e/0xd0
> > [17313.245992]  do_sys_open+0x184/0x220
> > [17313.245999]  do_syscall_64+0x5b/0x1b0
> >
> > Fixes: 487317c99477 ("cifs: add spinlock for the openFileList to cifsInodeInfo")
> >
> > CC: Stable <stable@vger.kernel.org>
> > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
> > Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > Signed-off-by: Steve French <stfrench@microsoft.com>
> >
> > diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> > index 4b95700c507c..3758237bf951 100644
> > --- a/fs/cifs/file.c
> > +++ b/fs/cifs/file.c
> > @@ -1840,13 +1840,12 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
> >  {
> >         struct cifsFileInfo *open_file = NULL;
> >         struct cifs_sb_info *cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb);
> > -       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> >
> >         /* only filter by fsuid on multiuser mounts */
> >         if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
> >                 fsuid_only = false;
> >
> > -       spin_lock(&tcon->open_file_lock);
> > +       spin_lock(&cifs_inode->open_file_lock);
> >         /* we could simply get the first_list_entry since write-only entries
> >            are always at the end of the list but since the first entry might
> >            have a close pending, we go through the whole list */
> > @@ -1858,7 +1857,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
> >                                 /* found a good file */
> >                                 /* lock it so it will not be closed on us */
> >                                 cifsFileInfo_get(open_file);
> > -                               spin_unlock(&tcon->open_file_lock);
> > +                               spin_unlock(&cifs_inode->open_file_lock);
> >                                 return open_file;
> >                         } /* else might as well continue, and look for
> >                              another, or simply have the caller reopen it
> > @@ -1866,7 +1865,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
> >                 } else /* write only file */
> >                         break; /* write only files are last so must be done */
> >         }
> > -       spin_unlock(&tcon->open_file_lock);
> > +       spin_unlock(&cifs_inode->open_file_lock);
> >         return NULL;
> >  }
> >
> > @@ -1877,7 +1876,6 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
> >  {
> >         struct cifsFileInfo *open_file, *inv_file = NULL;
> >         struct cifs_sb_info *cifs_sb;
> > -       struct cifs_tcon *tcon;
> >         bool any_available = false;
> >         int rc = -EBADF;
> >         unsigned int refind = 0;
> > @@ -1897,16 +1895,15 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
> >         }
> >
> >         cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb);
> > -       tcon = cifs_sb_master_tcon(cifs_sb);
> >
> >         /* only filter by fsuid on multiuser mounts */
> >         if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
> >                 fsuid_only = false;
> >
> > -       spin_lock(&tcon->open_file_lock);
> > +       spin_lock(&cifs_inode->open_file_lock);
> >  refind_writable:
> >         if (refind > MAX_REOPEN_ATT) {
> > -               spin_unlock(&tcon->open_file_lock);
> > +               spin_unlock(&cifs_inode->open_file_lock);
> >                 return rc;
> >         }
> >         list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
> > @@ -1918,7 +1915,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
> >                         if (!open_file->invalidHandle) {
> >                                 /* found a good writable file */
> >                                 cifsFileInfo_get(open_file);
> > -                               spin_unlock(&tcon->open_file_lock);
> > +                               spin_unlock(&cifs_inode->open_file_lock);
> >                                 *ret_file = open_file;
> >                                 return 0;
> >                         } else {
> > @@ -1938,7 +1935,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
> >                 cifsFileInfo_get(inv_file);
> >         }
> >
> > -       spin_unlock(&tcon->open_file_lock);
> > +       spin_unlock(&cifs_inode->open_file_lock);
> >
> >         if (inv_file) {
> >                 rc = cifs_reopen_file(inv_file, false);
> > @@ -1953,7 +1950,7 @@ cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only,
> >                 cifsFileInfo_put(inv_file);
> >                 ++refind;
> >                 inv_file = NULL;
> > -               spin_lock(&tcon->open_file_lock);
> > +               spin_lock(&cifs_inode->open_file_lock);
> >                 goto refind_writable;
> >         }
> >
> > @@ -4461,17 +4458,15 @@ static int cifs_readpage(struct file *file, struct page *page)
> >  static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
> >  {
> >         struct cifsFileInfo *open_file;
> > -       struct cifs_tcon *tcon =
> > -               cifs_sb_master_tcon(CIFS_SB(cifs_inode->vfs_inode.i_sb));
> >
> > -       spin_lock(&tcon->open_file_lock);
> > +       spin_lock(&cifs_inode->open_file_lock);
> >         list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
> >                 if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
> > -                       spin_unlock(&tcon->open_file_lock);
> > +                       spin_unlock(&cifs_inode->open_file_lock);
> >                         return 1;
> >                 }
> >         }
> > -       spin_unlock(&tcon->open_file_lock);
> > +       spin_unlock(&cifs_inode->open_file_lock);
> >         return 0;
> >  }
> >
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: FAILED: patch "[PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to" failed to apply to 4.19-stable tree
  2019-10-14 16:38 ` David Wysochanski
  2019-10-14 20:38   ` David Wysochanski
@ 2019-10-15  3:29   ` Sasha Levin
  2019-10-15  9:48     ` David Wysochanski
  1 sibling, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2019-10-15  3:29 UTC (permalink / raw)
  To: David Wysochanski; +Cc: gregkh, Ronnie Sahlberg, stable, stfrench

On Mon, Oct 14, 2019 at 12:38:57PM -0400, David Wysochanski wrote:
>Unless there is objections, let me try to fix this up.
>Thanks.

Hi David,

This was mostly due to missing fe768d51c832e ("CIFS: Return error code
when getting file handle for writeback"). I've adjusted the context and
queued it for 4.19. However, if you do end up fixing it up on your side
please share it with me and I'll confirm that I did the right thing.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: FAILED: patch "[PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to" failed to apply to 4.19-stable tree
  2019-10-14 20:38   ` David Wysochanski
@ 2019-10-15  5:24     ` Sasha Levin
  0 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2019-10-15  5:24 UTC (permalink / raw)
  To: David Wysochanski; +Cc: gregkh, Ronnie Sahlberg, stable, stfrench

On Mon, Oct 14, 2019 at 04:38:13PM -0400, David Wysochanski wrote:
>Original patch applies cleanly to 5.1-5.3. Prior to that (4.19-5.0),
>needs minor fixups.
>Do I need to re-send the original and the fixup with different kernel ranges?

Just patches that would apply on top of <=4.19.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: FAILED: patch "[PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to" failed to apply to 4.19-stable tree
  2019-10-15  3:29   ` Sasha Levin
@ 2019-10-15  9:48     ` David Wysochanski
  0 siblings, 0 replies; 6+ messages in thread
From: David Wysochanski @ 2019-10-15  9:48 UTC (permalink / raw)
  To: Sasha Levin; +Cc: gregkh, Ronnie Sahlberg, stable, stfrench

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Mon, Oct 14, 2019 at 11:29 PM Sasha Levin <sashal@kernel.org> wrote:
>
> On Mon, Oct 14, 2019 at 12:38:57PM -0400, David Wysochanski wrote:
> >Unless there is objections, let me try to fix this up.
> >Thanks.
>
> Hi David,
>
> This was mostly due to missing fe768d51c832e ("CIFS: Return error code
> when getting file handle for writeback"). I've adjusted the context and
> queued it for 4.19. However, if you do end up fixing it up on your side
> please share it with me and I'll confirm that I did the right thing.
>
> --
> Thanks,
> Sasha


See attached for my 4.19 backport so you can compare.

[-- Attachment #2: 0001-cifs-use-cifsInodeInfo-open_file_lock-while-iteratin.patch --]
[-- Type: text/x-patch, Size: 6978 bytes --]

From 1621c660af09098aa3ebc8ca78dd0c170f0f437b Mon Sep 17 00:00:00 2001
From: Dave Wysochanski <dwysocha@redhat.com>
Date: Thu, 3 Oct 2019 15:16:27 +1000
Subject: [PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to
 avoid a panic

Commit 487317c99477 ("cifs: add spinlock for the openFileList to
cifsInodeInfo") added cifsInodeInfo->open_file_lock spin_lock to protect
the openFileList, but missed a few places where cifs_inode->openFileList
was enumerated.  Change these remaining tcon->open_file_lock to
cifsInodeInfo->open_file_lock to avoid panic in is_size_safe_to_change.

[17313.245641] RIP: 0010:is_size_safe_to_change+0x57/0xb0 [cifs]
[17313.245645] Code: 68 40 48 89 ef e8 19 67 b7 f1 48 8b 43 40 48 8d 4b 40 48 8d 50 f0 48 39 c1 75 0f eb 47 48 8b 42 10 48 8d 50 f0 48 39 c1 74 3a <8b> 80 88 00 00 00 83 c0 01 a8 02 74 e6 48 89 ef c6 07 00 0f 1f 40
[17313.245649] RSP: 0018:ffff94ae1baefa30 EFLAGS: 00010202
[17313.245654] RAX: dead000000000100 RBX: ffff88dc72243300 RCX: ffff88dc72243340
[17313.245657] RDX: dead0000000000f0 RSI: 00000000098f7940 RDI: ffff88dd3102f040
[17313.245659] RBP: ffff88dd3102f040 R08: 0000000000000000 R09: ffff94ae1baefc40
[17313.245661] R10: ffffcdc8bb1c4e80 R11: ffffcdc8b50adb08 R12: 00000000098f7940
[17313.245663] R13: ffff88dc72243300 R14: ffff88dbc8f19600 R15: ffff88dc72243428
[17313.245667] FS:  00007fb145485700(0000) GS:ffff88dd3e000000(0000) knlGS:0000000000000000
[17313.245670] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[17313.245672] CR2: 0000026bb46c6000 CR3: 0000004edb110003 CR4: 00000000007606e0
[17313.245753] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[17313.245756] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[17313.245759] PKRU: 55555554
[17313.245761] Call Trace:
[17313.245803]  cifs_fattr_to_inode+0x16b/0x580 [cifs]
[17313.245838]  cifs_get_inode_info+0x35c/0xa60 [cifs]
[17313.245852]  ? kmem_cache_alloc_trace+0x151/0x1d0
[17313.245885]  cifs_open+0x38f/0x990 [cifs]
[17313.245921]  ? cifs_revalidate_dentry_attr+0x3e/0x350 [cifs]
[17313.245953]  ? cifsFileInfo_get+0x30/0x30 [cifs]
[17313.245960]  ? do_dentry_open+0x132/0x330
[17313.245963]  do_dentry_open+0x132/0x330
[17313.245969]  path_openat+0x573/0x14d0
[17313.245974]  do_filp_open+0x93/0x100
[17313.245979]  ? __check_object_size+0xa3/0x181
[17313.245986]  ? audit_alloc_name+0x7e/0xd0
[17313.245992]  do_sys_open+0x184/0x220
[17313.245999]  do_syscall_64+0x5b/0x1b0

Fixes: 487317c99477 ("cifs: add spinlock for the openFileList to cifsInodeInfo")

CC: Stable <stable@vger.kernel.org>
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
(cherry picked from commit cb248819d209d113e45fed459773991518e8e80b)

Conflicts:
	fs/cifs/file.c
---
 fs/cifs/file.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 8703b5f26f45..26027d85ffd5 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1835,13 +1835,12 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
 {
 	struct cifsFileInfo *open_file = NULL;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb);
-	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
 
 	/* only filter by fsuid on multiuser mounts */
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
 		fsuid_only = false;
 
-	spin_lock(&tcon->open_file_lock);
+	spin_lock(&cifs_inode->open_file_lock);
 	/* we could simply get the first_list_entry since write-only entries
 	   are always at the end of the list but since the first entry might
 	   have a close pending, we go through the whole list */
@@ -1853,7 +1852,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
 				/* found a good file */
 				/* lock it so it will not be closed on us */
 				cifsFileInfo_get(open_file);
-				spin_unlock(&tcon->open_file_lock);
+				spin_unlock(&cifs_inode->open_file_lock);
 				return open_file;
 			} /* else might as well continue, and look for
 			     another, or simply have the caller reopen it
@@ -1861,7 +1860,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
 		} else /* write only file */
 			break; /* write only files are last so must be done */
 	}
-	spin_unlock(&tcon->open_file_lock);
+	spin_unlock(&cifs_inode->open_file_lock);
 	return NULL;
 }
 
@@ -1870,7 +1869,6 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
 {
 	struct cifsFileInfo *open_file, *inv_file = NULL;
 	struct cifs_sb_info *cifs_sb;
-	struct cifs_tcon *tcon;
 	bool any_available = false;
 	int rc;
 	unsigned int refind = 0;
@@ -1886,16 +1884,15 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
 	}
 
 	cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb);
-	tcon = cifs_sb_master_tcon(cifs_sb);
 
 	/* only filter by fsuid on multiuser mounts */
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER))
 		fsuid_only = false;
 
-	spin_lock(&tcon->open_file_lock);
+	spin_lock(&cifs_inode->open_file_lock);
 refind_writable:
 	if (refind > MAX_REOPEN_ATT) {
-		spin_unlock(&tcon->open_file_lock);
+		spin_unlock(&cifs_inode->open_file_lock);
 		return NULL;
 	}
 	list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
@@ -1907,7 +1904,7 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
 			if (!open_file->invalidHandle) {
 				/* found a good writable file */
 				cifsFileInfo_get(open_file);
-				spin_unlock(&tcon->open_file_lock);
+				spin_unlock(&cifs_inode->open_file_lock);
 				return open_file;
 			} else {
 				if (!inv_file)
@@ -1926,7 +1923,7 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
 		cifsFileInfo_get(inv_file);
 	}
 
-	spin_unlock(&tcon->open_file_lock);
+	spin_unlock(&cifs_inode->open_file_lock);
 
 	if (inv_file) {
 		rc = cifs_reopen_file(inv_file, false);
@@ -1940,7 +1937,7 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
 			cifsFileInfo_put(inv_file);
 			++refind;
 			inv_file = NULL;
-			spin_lock(&tcon->open_file_lock);
+			spin_lock(&cifs_inode->open_file_lock);
 			goto refind_writable;
 		}
 	}
@@ -4001,17 +3998,15 @@ static int cifs_readpage(struct file *file, struct page *page)
 static int is_inode_writable(struct cifsInodeInfo *cifs_inode)
 {
 	struct cifsFileInfo *open_file;
-	struct cifs_tcon *tcon =
-		cifs_sb_master_tcon(CIFS_SB(cifs_inode->vfs_inode.i_sb));
 
-	spin_lock(&tcon->open_file_lock);
+	spin_lock(&cifs_inode->open_file_lock);
 	list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
 		if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
-			spin_unlock(&tcon->open_file_lock);
+			spin_unlock(&cifs_inode->open_file_lock);
 			return 1;
 		}
 	}
-	spin_unlock(&tcon->open_file_lock);
+	spin_unlock(&cifs_inode->open_file_lock);
 	return 0;
 }
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-15  9:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 16:18 FAILED: patch "[PATCH] cifs: use cifsInodeInfo->open_file_lock while iterating to" failed to apply to 4.19-stable tree gregkh
2019-10-14 16:38 ` David Wysochanski
2019-10-14 20:38   ` David Wysochanski
2019-10-15  5:24     ` Sasha Levin
2019-10-15  3:29   ` Sasha Levin
2019-10-15  9:48     ` David Wysochanski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.