Re: [PATCH] cifs: Return the appropriate error in cifs_sb_tlink instead of a generic error.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 1, 2020 at 1:57 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
>
> Ok. Checked the test. It does a multiuser mount of a file share. (I'm
> assuming this is using sec=ntlmssp)
> Then does an "ls" on the mount point as another user. Expects EACCES
> as the output.
> With this change, the command returns an ENOKEY.
> With the credentials added to keyring using cifscreds, the ls command works.
>
> Now the question is: Is the right error to return here EACCES or
> ENOKEY? Even if the expected error here should be EACCES, I'd say that
> the error returned by cifs_set_cifscreds should return that error.
>

Good find!

I am leaning towards leaving it as EACCES as that is a generic "you
dont have access" errno we report back to userspace.
System calls like opendir(), stat() etc all list EACCES as a valid
return code but they don't list ENOKEY.
So we then have to consider the applications. For applications that
don't look at errno it wouldn't matter but for applications that do
look at errno and try to take proper action to failures would be
"surprised" since they get an errno that is not listed on the manpage.

So that is why I think we should leave it as -EACCES as this is the
generic "you don't have access".
But we should make sure that when this happens we do get a more
detailed "you don't have access because you don't have proper
credentials" into log-file / dmesg.

Regards
Ronnie

> Regards,
> Shyam
>
> On Wed, Sep 30, 2020 at 8:48 PM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
> >
> > It looks like xfstests smb3 multiuser cifsutils-101 and cifs-101 are failing.
> > Maybe they were written keeping in mind the current error code
> > returned by cifs.ko in this situation? Let me take a look.
> > I guess @ronnie sahlberg will be able to debug the failing tests
> > faster. The failing test has his name in the code. :)
> >
> > On Tue, Sep 29, 2020 at 11:16 PM Steve French <smfrench@xxxxxxxxx> wrote:
> > >
> > > tentatively merged ... running the usual functional tests
> > > http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/399
> > >
> > > On Tue, Sep 29, 2020 at 8:16 AM Aurélien Aptel <aaptel@xxxxxxxx> wrote:
> > > >
> > > > Shyam Prasad N <nspmangalore@xxxxxxxxx> writes:
> > > > > One of the cases where this behaviour is confusing is where a
> > > > > new tcon needs to be constructed, but it fails due to
> > > > > expired keys. cifs_construct_tcon then returns ENOKEY,
> > > > > but we end up returning a EACCES to the user.
> > > > >
> > > > > Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
> > > >
> > > > LGTM
> > > >
> > > > Reviewed-by: Aurelien Aptel <aaptel@xxxxxxxx>
> > > >
> > > > Cheers,
> > > > --
> > > > Aurélien Aptel / SUSE Labs Samba Team
> > > > GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> > > > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> > > > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)
> > >
> > >
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
> >
> >
> >
> > --
> > -Shyam
>
>
>
> --
> -Shyam




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux