Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL

Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL

[Pavel Shilovsky]

чт, 25 июл. 2019 г. в 09:57, Sebastien Tisserant via samba-technical
<samba-technical@lists.samba.org>:
...
>
> mount works without CONFIG_DEBUG_VIRTUAL
>
> If we don't set CONFIG_VMAP_STACK mount works with CONFIG_DEBUG_VIRTUAL
>
>
> We have the following (very quick and dirty) patch :
>
> Index: linux-4.19.60/fs/cifs/smb2ops.c
> ===================================================================
> --- linux-4.19.60.orig/fs/cifs/smb2ops.c
> +++ linux-4.19.60/fs/cifs/smb2ops.c
> @@ -2545,7 +2545,15 @@ fill_transform_hdr(struct smb2_transform
>  static inline void smb2_sg_set_buf(struct scatterlist *sg, const void *buf,
>                     unsigned int buflen)
>  {
> -    sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
> +      void *addr;
> +      /*
> +       * VMAP_STACK (at least) puts stack into the vmalloc address space
> +      */
> +      if (is_vmalloc_addr(buf))
> +              addr = vmalloc_to_page(buf);
> +      else
> +              addr = virt_to_page(buf);
> +      sg_set_page(sg, addr, buflen, offset_in_page(buf));
>  }
>
>  /* Assumes the first rqst has a transform header as the first iov.
>
>

Thanks for reporting this. The patch looks good to me. Did you test
your scenario all together with it (not only mounting)?


Best regards,
Pavel Shilovskiy

Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL

[Cyrille Mucchietto]

On 7/25/19 10:35 PM, Pavel Shilovsky wrote:
...
>> mount works without CONFIG_DEBUG_VIRTUAL
>>
>> If we don't set CONFIG_VMAP_STACK mount works with CONFIG_DEBUG_VIRTUAL
>> We have the following (very quick and dirty) patch :
>> ...
> Thanks for reporting this. The patch looks good to me. Did you test
> your scenario all together with it (not only mounting)?
>
>
> Best regards,
> Pavel Shilovskiy

We've done some tests (mount/unmount, copy/delete/move files) and it seems
to be OK.

We aren't sure that our patch corrects the root cause of this issue, as
for our
understanding, kernel stack with VMAP_STACK enabled might not be contiguous.
If the stack is too big to be on one page and data are splitted accross non
contiguous pages it might not work.

Regards

Cyrille Mucchietto

Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL

[Steve French]

If the patch is an improvement - I am ok with it.  Unless one of the
mm experts has better idea

On Fri, Jul 26, 2019 at 1:28 AM Cyrille Mucchietto
<cmucchietto@wallix.com> wrote:
>
> On 7/25/19 10:35 PM, Pavel Shilovsky wrote:
> ...
> >> mount works without CONFIG_DEBUG_VIRTUAL
> >>
> >> If we don't set CONFIG_VMAP_STACK mount works with CONFIG_DEBUG_VIRTUAL
> >> We have the following (very quick and dirty) patch :
> >> ...
> > Thanks for reporting this. The patch looks good to me. Did you test
> > your scenario all together with it (not only mounting)?
> >
> >
> > Best regards,
> > Pavel Shilovskiy
>
> We've done some tests (mount/unmount, copy/delete/move files) and it seems
> to be OK.
>
> We aren't sure that our patch corrects the root cause of this issue, as
> for our
> understanding, kernel stack with VMAP_STACK enabled might not be contiguous.
> If the stack is too big to be on one page and data are splitted accross non
> contiguous pages it might not work.
>
> Regards
>
> Cyrille Mucchietto
>
>


-- 
Thanks,

Steve

Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL

[Steve French]

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

Sebastien,
I cleaned up the patch and merged into cifs-2.6.git - can you
doublecheck it is correct?


On Thu, Jul 25, 2019 at 3:35 PM Pavel Shilovsky
<pavel.shilovsky@gmail.com> wrote:
>
> чт, 25 июл. 2019 г. в 09:57, Sebastien Tisserant via samba-technical
> <samba-technical@lists.samba.org>:
> ...
> >
> > mount works without CONFIG_DEBUG_VIRTUAL
> >
> > If we don't set CONFIG_VMAP_STACK mount works with CONFIG_DEBUG_VIRTUAL
> >
> >
> > We have the following (very quick and dirty) patch :
> >
> > Index: linux-4.19.60/fs/cifs/smb2ops.c
> > ===================================================================
> > --- linux-4.19.60.orig/fs/cifs/smb2ops.c
> > +++ linux-4.19.60/fs/cifs/smb2ops.c
> > @@ -2545,7 +2545,15 @@ fill_transform_hdr(struct smb2_transform
> >  static inline void smb2_sg_set_buf(struct scatterlist *sg, const void *buf,
> >                     unsigned int buflen)
> >  {
> > -    sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
> > +      void *addr;
> > +      /*
> > +       * VMAP_STACK (at least) puts stack into the vmalloc address space
> > +      */
> > +      if (is_vmalloc_addr(buf))
> > +              addr = vmalloc_to_page(buf);
> > +      else
> > +              addr = virt_to_page(buf);
> > +      sg_set_page(sg, addr, buflen, offset_in_page(buf));
> >  }
> >
> >  /* Assumes the first rqst has a transform header as the first iov.
> >
> >
>
> Thanks for reporting this. The patch looks good to me. Did you test
> your scenario all together with it (not only mounting)?
>
>
> Best regards,
> Pavel Shilovskiy



-- 
Thanks,

Steve

[-- Attachment #2: 0001-SMB3-Kernel-oops-mounting-a-encryptData-share-with-C.patch --]
[-- Type: text/x-patch, Size: 1276 bytes --]

From e7afda65be3198639de0fcf1796ddd0a101e1ab1 Mon Sep 17 00:00:00 2001
From: Sebastien Tisserant <stisserant@wallix.com>
Date: Thu, 1 Aug 2019 12:06:08 -0500
Subject: [PATCH] SMB3: Kernel oops mounting a encryptData share with
 CONFIG_DEBUG_VIRTUAL

Fix kernel oops when mounting a encryptData CIFS share with
CONFIG_DEBUG_VIRTUAL

Signed-off-by: Sebastien Tisserant <stisserant@wallix.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/smb2ops.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index fc464dc20b30..3e3243d7152c 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3510,7 +3510,15 @@ fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len,
 static inline void smb2_sg_set_buf(struct scatterlist *sg, const void *buf,
 				   unsigned int buflen)
 {
-	sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
+	void *addr;
+	/*
+	 * VMAP_STACK (at least) puts stack into the vmalloc address space
+	 */
+	if (is_vmalloc_addr(buf))
+		addr = vmalloc_to_page(buf);
+	else
+		addr = virt_to_page(buf);
+	sg_set_page(sg, addr, buflen, offset_in_page(buf));
 }
 
 /* Assumes the first rqst has a transform header as the first iov.
-- 
2.20.1

Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL

[Cyrille Mucchietto]

On 8/1/19 7:13 PM, Steve French wrote:
> Sebastien,
> I cleaned up the patch and merged into cifs-2.6.git - can you
> doublecheck it is correct?

Sebastien is not avalaible, but we confirm that this patch is correct

Regards,

Cyrille Mucchietto


> On Thu, Jul 25, 2019 at 3:35 PM Pavel Shilovsky
> <pavel.shilovsky@gmail.com> wrote:
>> чт, 25 июл. 2019 г. в 09:57, Sebastien Tisserant via samba-technical
>> <samba-technical@lists.samba.org>:
>> ...
>>> mount works without CONFIG_DEBUG_VIRTUAL
>>>
>>> If we don't set CONFIG_VMAP_STACK mount works with CONFIG_DEBUG_VIRTUAL
>>>
>>>
>>> We have the following (very quick and dirty) patch :
>>>
>>> Index: linux-4.19.60/fs/cifs/smb2ops.c
>>> ===================================================================
>>> --- linux-4.19.60.orig/fs/cifs/smb2ops.c
>>> +++ linux-4.19.60/fs/cifs/smb2ops.c
>>> @@ -2545,7 +2545,15 @@ fill_transform_hdr(struct smb2_transform
>>>  static inline void smb2_sg_set_buf(struct scatterlist *sg, const void *buf,
>>>                     unsigned int buflen)
>>>  {
>>> -    sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
>>> +      void *addr;
>>> +      /*
>>> +       * VMAP_STACK (at least) puts stack into the vmalloc address space
>>> +      */
>>> +      if (is_vmalloc_addr(buf))
>>> +              addr = vmalloc_to_page(buf);
>>> +      else
>>> +              addr = virt_to_page(buf);
>>> +      sg_set_page(sg, addr, buflen, offset_in_page(buf));
>>>  }
>>>
>>>  /* Assumes the first rqst has a transform header as the first iov.
>>>
>>>
>> Thanks for reporting this. The patch looks good to me. Did you test
>> your scenario all together with it (not only mounting)?
>>
>>
>> Best regards,
>> Pavel Shilovskiy
>>
>>