diff mbox series

cifs: fix regression in very old smb1 mounts

Message ID 20221011231207.1458541-1-lsahlber@redhat.com
State New
Headers show
Series cifs: fix regression in very old smb1 mounts | expand

Commit Message

Ronnie Sahlberg Oct. 11, 2022, 11:12 p.m. UTC
BZ: 215375

Fixes: 76a3c92ec9e0668e4cd0e9ff1782eb68f61a179c ("cifs: remove support for NTLM and weaker authentication algorithms")
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/connect.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Thorsten Leemhuis Nov. 1, 2022, 5:53 p.m. UTC | #1
On 12.10.22 01:12, Ronnie Sahlberg wrote:
> BZ: 215375
> 
> Fixes: 76a3c92ec9e0668e4cd0e9ff1782eb68f61a179c ("cifs: remove support for NTLM and weaker authentication algorithms")
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>

Ronnie, Steve, did this change create any trouble in 6.1 pre-releases so
far? If not: could you please consider submitting this change for
inclusion in 6.0 and 5.15, as this is a regression from the 5.15 days
that according to the bugzilla ticket seem to annoy some people a lot.

Ciao, Thorsten

> ---
>  fs/cifs/connect.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 93e59b3b36c7..c77232096c12 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3922,12 +3922,11 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>  	pSMB->AndXCommand = 0xFF;
>  	pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
>  	bcc_ptr = &pSMB->Password[0];
> -	if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
> -		pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
> -		*bcc_ptr = 0; /* password is null byte */
> -		bcc_ptr++;              /* skip password */
> -		/* already aligned so no need to do it below */
> -	}
> +
> +	pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
> +	*bcc_ptr = 0; /* password is null byte */
> +	bcc_ptr++;              /* skip password */
> +	/* already aligned so no need to do it below */
>  
>  	if (ses->server->sign)
>  		smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
Thorsten Leemhuis Nov. 7, 2022, 1:17 p.m. UTC | #2
On 01.11.22 18:53, Thorsten Leemhuis wrote:
> On 12.10.22 01:12, Ronnie Sahlberg wrote:
>> BZ: 215375
>>
>> Fixes: 76a3c92ec9e0668e4cd0e9ff1782eb68f61a179c ("cifs: remove support for NTLM and weaker authentication algorithms")
>> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> 
> Ronnie, Steve, did this change create any trouble in 6.1 pre-releases so
> far? If not: could you please consider submitting this change for
> inclusion in 6.0 and 5.15, as this is a regression from the 5.15 days
> that according to the bugzilla ticket seem to annoy some people a lot.

Ronnie, Steve, if you have a minute, I would really appreciate your help
in this matter, you are the best people to judge here.

Ciao, Thorsten

>> ---
>>  fs/cifs/connect.c | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 93e59b3b36c7..c77232096c12 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -3922,12 +3922,11 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>>  	pSMB->AndXCommand = 0xFF;
>>  	pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
>>  	bcc_ptr = &pSMB->Password[0];
>> -	if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
>> -		pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
>> -		*bcc_ptr = 0; /* password is null byte */
>> -		bcc_ptr++;              /* skip password */
>> -		/* already aligned so no need to do it below */
>> -	}
>> +
>> +	pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
>> +	*bcc_ptr = 0; /* password is null byte */
>> +	bcc_ptr++;              /* skip password */
>> +	/* already aligned so no need to do it below */
>>  
>>  	if (ses->server->sign)
>>  		smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
ronnie sahlberg Nov. 7, 2022, 1:31 p.m. UTC | #3
On Mon, 7 Nov 2022 at 23:20, Thorsten Leemhuis
<regressions@leemhuis.info> wrote:
>
> On 01.11.22 18:53, Thorsten Leemhuis wrote:
> > On 12.10.22 01:12, Ronnie Sahlberg wrote:
> >> BZ: 215375
> >>
> >> Fixes: 76a3c92ec9e0668e4cd0e9ff1782eb68f61a179c ("cifs: remove support for NTLM and weaker authentication algorithms")
> >> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> >
> > Ronnie, Steve, did this change create any trouble in 6.1 pre-releases so
> > far? If not: could you please consider submitting this change for
> > inclusion in 6.0 and 5.15, as this is a regression from the 5.15 days
> > that according to the bugzilla ticket seem to annoy some people a lot.
>
> Ronnie, Steve, if you have a minute, I would really appreciate your help
> in this matter, you are the best people to judge here.

Thanks for the reminder.  I don't think there were any issues in the
pre-release so we should try to get it into the stable kernels.
I am not sure how that process works since the patch is already in upstream.
(I have only seen the process where you flag the patch with cc-stable.)
Can you explain the process on how to flag this existing patch for backporting?


>
> Ciao, Thorsten
>
> >> ---
> >>  fs/cifs/connect.c | 11 +++++------
> >>  1 file changed, 5 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> >> index 93e59b3b36c7..c77232096c12 100644
> >> --- a/fs/cifs/connect.c
> >> +++ b/fs/cifs/connect.c
> >> @@ -3922,12 +3922,11 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
> >>      pSMB->AndXCommand = 0xFF;
> >>      pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
> >>      bcc_ptr = &pSMB->Password[0];
> >> -    if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
> >> -            pSMB->PasswordLength = cpu_to_le16(1);  /* minimum */
> >> -            *bcc_ptr = 0; /* password is null byte */
> >> -            bcc_ptr++;              /* skip password */
> >> -            /* already aligned so no need to do it below */
> >> -    }
> >> +
> >> +    pSMB->PasswordLength = cpu_to_le16(1);  /* minimum */
> >> +    *bcc_ptr = 0; /* password is null byte */
> >> +    bcc_ptr++;              /* skip password */
> >> +    /* already aligned so no need to do it below */
> >>
> >>      if (ses->server->sign)
> >>              smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
Thorsten Leemhuis Nov. 7, 2022, 1:40 p.m. UTC | #4
On 07.11.22 14:31, ronnie sahlberg wrote:
> On Mon, 7 Nov 2022 at 23:20, Thorsten Leemhuis
> <regressions@leemhuis.info> wrote:
>>
>> On 01.11.22 18:53, Thorsten Leemhuis wrote:
>>> On 12.10.22 01:12, Ronnie Sahlberg wrote:
>>>> BZ: 215375
>>>>
>>>> Fixes: 76a3c92ec9e0668e4cd0e9ff1782eb68f61a179c ("cifs: remove support for NTLM and weaker authentication algorithms")
>>>> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
>>>
>>> Ronnie, Steve, did this change create any trouble in 6.1 pre-releases so
>>> far? If not: could you please consider submitting this change for
>>> inclusion in 6.0 and 5.15, as this is a regression from the 5.15 days
>>> that according to the bugzilla ticket seem to annoy some people a lot.
>>
>> Ronnie, Steve, if you have a minute, I would really appreciate your help
>> in this matter, you are the best people to judge here.
> 
> Thanks for the reminder.  I don't think there were any issues in the
> pre-release so we should try to get it into the stable kernels.

Great.

> I am not sure how that process works since the patch is already in upstream.
> (I have only seen the process where you flag the patch with cc-stable.)
> Can you explain the process on how to flag this existing patch for backporting?

Documentation/process/stable-kernel-rules.rst (or
https://docs.kernel.org/process/stable-kernel-rules.html ) explains this
(see option 3 there) better than I can. The patch afaics needs to got to
6.0 and 5.15.

Many thx for taking care of this!

Ciao, Thorsten


>>>> ---
>>>>  fs/cifs/connect.c | 11 +++++------
>>>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>>>> index 93e59b3b36c7..c77232096c12 100644
>>>> --- a/fs/cifs/connect.c
>>>> +++ b/fs/cifs/connect.c
>>>> @@ -3922,12 +3922,11 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
>>>>      pSMB->AndXCommand = 0xFF;
>>>>      pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
>>>>      bcc_ptr = &pSMB->Password[0];
>>>> -    if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
>>>> -            pSMB->PasswordLength = cpu_to_le16(1);  /* minimum */
>>>> -            *bcc_ptr = 0; /* password is null byte */
>>>> -            bcc_ptr++;              /* skip password */
>>>> -            /* already aligned so no need to do it below */
>>>> -    }
>>>> +
>>>> +    pSMB->PasswordLength = cpu_to_le16(1);  /* minimum */
>>>> +    *bcc_ptr = 0; /* password is null byte */
>>>> +    bcc_ptr++;              /* skip password */
>>>> +    /* already aligned so no need to do it below */
>>>>
>>>>      if (ses->server->sign)
>>>>              smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
>
ronnie sahlberg Nov. 7, 2022, 9:40 p.m. UTC | #5
On Mon, 7 Nov 2022 at 23:40, Thorsten Leemhuis
<regressions@leemhuis.info> wrote:
>
> On 07.11.22 14:31, ronnie sahlberg wrote:
> > On Mon, 7 Nov 2022 at 23:20, Thorsten Leemhuis
> > <regressions@leemhuis.info> wrote:
> >>
> >> On 01.11.22 18:53, Thorsten Leemhuis wrote:
> >>> On 12.10.22 01:12, Ronnie Sahlberg wrote:
> >>>> BZ: 215375
> >>>>
> >>>> Fixes: 76a3c92ec9e0668e4cd0e9ff1782eb68f61a179c ("cifs: remove support for NTLM and weaker authentication algorithms")
> >>>> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> >>>
> >>> Ronnie, Steve, did this change create any trouble in 6.1 pre-releases so
> >>> far? If not: could you please consider submitting this change for
> >>> inclusion in 6.0 and 5.15, as this is a regression from the 5.15 days
> >>> that according to the bugzilla ticket seem to annoy some people a lot.
> >>
> >> Ronnie, Steve, if you have a minute, I would really appreciate your help
> >> in this matter, you are the best people to judge here.
> >
> > Thanks for the reminder.  I don't think there were any issues in the
> > pre-release so we should try to get it into the stable kernels.
>
> Great.
>
> > I am not sure how that process works since the patch is already in upstream.
> > (I have only seen the process where you flag the patch with cc-stable.)
> > Can you explain the process on how to flag this existing patch for backporting?
>
> Documentation/process/stable-kernel-rules.rst (or
> https://docs.kernel.org/process/stable-kernel-rules.html ) explains this
> (see option 3 there) better than I can. The patch afaics needs to got to
> 6.0 and 5.15.

Thanks. What we need here is option 2.
Steve, can you send an email for option 2 so we get it in the older kernels?


>
> Many thx for taking care of this!
>
> Ciao, Thorsten
>
>
> >>>> ---
> >>>>  fs/cifs/connect.c | 11 +++++------
> >>>>  1 file changed, 5 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> >>>> index 93e59b3b36c7..c77232096c12 100644
> >>>> --- a/fs/cifs/connect.c
> >>>> +++ b/fs/cifs/connect.c
> >>>> @@ -3922,12 +3922,11 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
> >>>>      pSMB->AndXCommand = 0xFF;
> >>>>      pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
> >>>>      bcc_ptr = &pSMB->Password[0];
> >>>> -    if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
> >>>> -            pSMB->PasswordLength = cpu_to_le16(1);  /* minimum */
> >>>> -            *bcc_ptr = 0; /* password is null byte */
> >>>> -            bcc_ptr++;              /* skip password */
> >>>> -            /* already aligned so no need to do it below */
> >>>> -    }
> >>>> +
> >>>> +    pSMB->PasswordLength = cpu_to_le16(1);  /* minimum */
> >>>> +    *bcc_ptr = 0; /* password is null byte */
> >>>> +    bcc_ptr++;              /* skip password */
> >>>> +    /* already aligned so no need to do it below */
> >>>>
> >>>>      if (ses->server->sign)
> >>>>              smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;
> >
diff mbox series

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 93e59b3b36c7..c77232096c12 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3922,12 +3922,11 @@  CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
 	pSMB->AndXCommand = 0xFF;
 	pSMB->Flags = cpu_to_le16(TCON_EXTENDED_SECINFO);
 	bcc_ptr = &pSMB->Password[0];
-	if (tcon->pipe || (ses->server->sec_mode & SECMODE_USER)) {
-		pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
-		*bcc_ptr = 0; /* password is null byte */
-		bcc_ptr++;              /* skip password */
-		/* already aligned so no need to do it below */
-	}
+
+	pSMB->PasswordLength = cpu_to_le16(1);	/* minimum */
+	*bcc_ptr = 0; /* password is null byte */
+	bcc_ptr++;              /* skip password */
+	/* already aligned so no need to do it below */
 
 	if (ses->server->sign)
 		smb_buffer->Flags2 |= SMBFLG2_SECURITY_SIGNATURE;