Hi Ronnie, Returning EACCES to userspace does make sense. However, with the current code, even a ENOMEM or EIO will return a EACCES to userspace. I'm thinking that we need to specifically map ENOKEY to EACCES here. I was discussing this with Steve yesterday, and he suggested that I email fs-devel about what is the general expectation from VFS. Regards, Shyam On Thu, Oct 1, 2020 at 4:47 AM ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote: > > 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 -- -Shyam
