Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL
       [not found] <380e1b86-1911-b8a5-6b02-276b6d4be4fe@wallix.com>
@ 2019-07-25 20:35 ` Pavel Shilovsky
  2019-07-26  6:28   ` Cyrille Mucchietto
  2019-08-01 17:13   ` Steve French
  0 siblings, 2 replies; 5+ messages in thread
From: Pavel Shilovsky @ 2019-07-25 20:35 UTC (permalink / raw)
  To: Sebastien Tisserant
  Cc: Steve French, linux-cifs, samba-technical, Cristian Popi,
	Cyrille Mucchietto

чт, 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

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

* Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL
  2019-07-25 20:35 ` PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL Pavel Shilovsky
@ 2019-07-26  6:28   ` Cyrille Mucchietto
  2019-07-29 21:02     ` Steve French
  2019-08-01 17:13   ` Steve French
  1 sibling, 1 reply; 5+ messages in thread
From: Cyrille Mucchietto @ 2019-07-26  6:28 UTC (permalink / raw)
  To: Pavel Shilovsky, Sebastien Tisserant
  Cc: Steve French, linux-cifs, samba-technical, Cristian Popi

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



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

* Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL
  2019-07-26  6:28   ` Cyrille Mucchietto
@ 2019-07-29 21:02     ` Steve French
  0 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2019-07-29 21:02 UTC (permalink / raw)
  To: Cyrille Mucchietto
  Cc: Pavel Shilovsky, Sebastien Tisserant, Steve French, linux-cifs,
	samba-technical, Cristian Popi

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

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

* Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL
  2019-07-25 20:35 ` PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL Pavel Shilovsky
  2019-07-26  6:28   ` Cyrille Mucchietto
@ 2019-08-01 17:13   ` Steve French
  2019-08-02  6:58     ` Cyrille Mucchietto
  1 sibling, 1 reply; 5+ messages in thread
From: Steve French @ 2019-08-01 17:13 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: Sebastien Tisserant, Steve French, linux-cifs, samba-technical,
	Cristian Popi, Cyrille Mucchietto

[-- 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


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

* Re: PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL
  2019-08-01 17:13   ` Steve French
@ 2019-08-02  6:58     ` Cyrille Mucchietto
  0 siblings, 0 replies; 5+ messages in thread
From: Cyrille Mucchietto @ 2019-08-02  6:58 UTC (permalink / raw)
  To: Steve French, Pavel Shilovsky
  Cc: Sebastien Tisserant, Steve French, linux-cifs, samba-technical,
	Cristian Popi

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
>>
>>

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <380e1b86-1911-b8a5-6b02-276b6d4be4fe@wallix.com>
2019-07-25 20:35 ` PROBLEM: Kernel oops when mounting a encryptData CIFS share with CONFIG_DEBUG_VIRTUAL Pavel Shilovsky
2019-07-26  6:28   ` Cyrille Mucchietto
2019-07-29 21:02     ` Steve French
2019-08-01 17:13   ` Steve French
2019-08-02  6:58     ` Cyrille Mucchietto

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org linux-cifs@archiver.kernel.org
	public-inbox-index linux-cifs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox