linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steve French <smfrench@gmail.com>
To: Ronnie Sahlberg <lsahlber@redhat.com>
Cc: linux-cifs <linux-cifs@vger.kernel.org>
Subject: Re: [PATCH] cifs: Do not leak EDEADLK to dgetents64 for STATUS_USER_SESSION_DELETED
Date: Wed, 25 Aug 2021 16:09:47 -0500	[thread overview]
Message-ID: <CAH2r5ms2KzVf-7ei2+m_GbwcvZ7PHTCbv33bhNaH9dXXWcVO1w@mail.gmail.com> (raw)
In-Reply-To: <20210825111656.1635954-2-lsahlber@redhat.com>

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

lightly updated to add short sleep before retry


On Wed, Aug 25, 2021 at 6:17 AM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ: 1994393
>
> If we hit a STATUS_USER_SESSION_DELETED for the Create part in the
> Create/QueryDirectory compound that starts a directory scan
> we will leak EDEADLK back to userspace and surprise glibc and the application.
>
> Pick this up initiate_cifs_search() and retry a small number of tries before we
> return an error to userspace.
>
> Cc: stable@vger.kernel.org
> Reported-by: Xiaoli Feng <xifeng@redhat.com>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/readdir.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
> index bfee176b901d..4518e3ca64df 100644
> --- a/fs/cifs/readdir.c
> +++ b/fs/cifs/readdir.c
> @@ -369,7 +369,7 @@ int get_symlink_reparse_path(char *full_path, struct cifs_sb_info *cifs_sb,
>   */
>
>  static int
> -initiate_cifs_search(const unsigned int xid, struct file *file,
> +_initiate_cifs_search(const unsigned int xid, struct file *file,
>                      const char *full_path)
>  {
>         __u16 search_flags;
> @@ -451,6 +451,23 @@ initiate_cifs_search(const unsigned int xid, struct file *file,
>         return rc;
>  }
>
> +static int
> +initiate_cifs_search(const unsigned int xid, struct file *file,
> +                    const char *full_path)
> +{
> +       int rc, retry_count = 0;
> +
> +       do {
> +               rc = _initiate_cifs_search(xid, file, full_path);
> +               /*
> +                * We don't have enough credits to start reading the
> +                * directory so just try again.
> +                */
> +       } while (rc == -EDEADLK && retry_count++ < 5);
> +
> +       return rc;
> +}
> +
>  /* return length of unicode string in bytes */
>  static int cifs_unicode_bytelen(const char *str)
>  {
> --
> 2.30.2
>


-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-Do-not-leak-EDEADLK-to-dgetents64-for-STATUS_US.patch --]
[-- Type: text/x-patch, Size: 1950 bytes --]

From 57cea50fa5a30068752a8155e1c7230c8c585493 Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <lsahlber@redhat.com>
Date: Wed, 25 Aug 2021 21:16:56 +1000
Subject: [PATCH] cifs: Do not leak EDEADLK to dgetents64 for
 STATUS_USER_SESSION_DELETED

RHBZ: 1994393

If we hit a STATUS_USER_SESSION_DELETED for the Create part in the
Create/QueryDirectory compound that starts a directory scan
we will leak EDEADLK back to userspace and surprise glibc and the application.

Pick this up initiate_cifs_search() and retry a small number of tries before we
return an error to userspace.

Cc: stable@vger.kernel.org
Reported-by: Xiaoli Feng <xifeng@redhat.com>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/readdir.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index bfee176b901d..54d77c99e21c 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -369,7 +369,7 @@ int get_symlink_reparse_path(char *full_path, struct cifs_sb_info *cifs_sb,
  */
 
 static int
-initiate_cifs_search(const unsigned int xid, struct file *file,
+_initiate_cifs_search(const unsigned int xid, struct file *file,
 		     const char *full_path)
 {
 	__u16 search_flags;
@@ -451,6 +451,27 @@ initiate_cifs_search(const unsigned int xid, struct file *file,
 	return rc;
 }
 
+static int
+initiate_cifs_search(const unsigned int xid, struct file *file,
+		     const char *full_path)
+{
+	int rc, retry_count = 0;
+
+	do {
+		rc = _initiate_cifs_search(xid, file, full_path);
+		/*
+		 * If we don't have enough credits to start reading the
+		 * directory just try again after short wait.
+		 */
+		if (rc != -EDEADLK)
+			break;
+
+		usleep_range(512, 2048);
+	} while (retry_count++ < 5);
+
+	return rc;
+}
+
 /* return length of unicode string in bytes */
 static int cifs_unicode_bytelen(const char *str)
 {
-- 
2.30.2


  reply	other threads:[~2021-08-25 21:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-25 11:16 [PATCH 0/1] don't leak EDEADLK to userspace Ronnie Sahlberg
2021-08-25 11:16 ` [PATCH] cifs: Do not leak EDEADLK to dgetents64 for STATUS_USER_SESSION_DELETED Ronnie Sahlberg
2021-08-25 21:09   ` Steve French [this message]
2021-08-26 17:15     ` Shyam Prasad N
2021-08-26 18:07       ` ronnie sahlberg
  -- strict thread matches above, loose matches on Subject: below --
2021-08-25  7:30 [PATCH 0/1] cifs: do not leak EDEADLK to dgetents64 Ronnie Sahlberg
2021-08-25  7:30 ` [PATCH] cifs: Do not leak EDEADLK to dgetents64 for STATUS_USER_SESSION_DELETED Ronnie Sahlberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAH2r5ms2KzVf-7ei2+m_GbwcvZ7PHTCbv33bhNaH9dXXWcVO1w@mail.gmail.com \
    --to=smfrench@gmail.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=lsahlber@redhat.com \
    --subject='Re: [PATCH] cifs: Do not leak EDEADLK to dgetents64 for STATUS_USER_SESSION_DELETED' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).