| Message ID | 20220213224052.3387192-2-lsahlber@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | cifs: modefromsids must add an ACE for authenticated users | expand |
Should I add: cc:Stable # 5.12+ Thoughts? On Sun, Feb 13, 2022 at 4:41 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote: > > When we create a file with modefromsids we set an ACL that > has one ACE for the magic modefromsid as well as a second ACE that > grants full access to all authenticated users. > > When later we chante the mode on the file we strip away this, and other, > ACE for authenticated users in set_chmod_dacl() and then just add back/update > the modefromsid ACE. > Thus leaving the file with a single ACE that is for the mode and no ACE > to grant any user any rights to access the file. > Fix this by always adding back also the modefromsid ACE so that we do not > drop the rights to access the file. > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/cifsacl.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c > index ee3aab3dd4ac..40cda87ce384 100644 > --- a/fs/cifs/cifsacl.c > +++ b/fs/cifs/cifsacl.c > @@ -949,6 +949,9 @@ static void populate_new_aces(char *nacl_base, > pnntace = (struct cifs_ace *) (nacl_base + nsize); > nsize += setup_special_mode_ACE(pnntace, nmode); > num_aces++; > + pnntace = (struct cifs_ace *) (nacl_base + nsize); > + nsize += setup_authusers_ACE(pnntace); > + num_aces++; > goto set_size; > } > > @@ -1613,7 +1616,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode, > nsecdesclen = secdesclen; > if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */ > if (mode_from_sid) > - nsecdesclen += sizeof(struct cifs_ace); > + nsecdesclen += 2 * sizeof(struct cifs_ace); > else /* cifsacl */ > nsecdesclen += 5 * sizeof(struct cifs_ace); > } else { /* chown */ > -- > 2.30.2 >
merged into cifs-2.6.git for-next pending review and more testing Ronnie, Maybe we should add a small test for create file with modefromsid, chmod the file, and then getcifsacl? On Sun, Feb 13, 2022 at 4:41 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote: > > When we create a file with modefromsids we set an ACL that > has one ACE for the magic modefromsid as well as a second ACE that > grants full access to all authenticated users. > > When later we chante the mode on the file we strip away this, and other, > ACE for authenticated users in set_chmod_dacl() and then just add back/update > the modefromsid ACE. > Thus leaving the file with a single ACE that is for the mode and no ACE > to grant any user any rights to access the file. > Fix this by always adding back also the modefromsid ACE so that we do not > drop the rights to access the file. > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/cifsacl.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c > index ee3aab3dd4ac..40cda87ce384 100644 > --- a/fs/cifs/cifsacl.c > +++ b/fs/cifs/cifsacl.c > @@ -949,6 +949,9 @@ static void populate_new_aces(char *nacl_base, > pnntace = (struct cifs_ace *) (nacl_base + nsize); > nsize += setup_special_mode_ACE(pnntace, nmode); > num_aces++; > + pnntace = (struct cifs_ace *) (nacl_base + nsize); > + nsize += setup_authusers_ACE(pnntace); > + num_aces++; > goto set_size; > } > > @@ -1613,7 +1616,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode, > nsecdesclen = secdesclen; > if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */ > if (mode_from_sid) > - nsecdesclen += sizeof(struct cifs_ace); > + nsecdesclen += 2 * sizeof(struct cifs_ace); > else /* cifsacl */ > nsecdesclen += 5 * sizeof(struct cifs_ace); > } else { /* chown */ > -- > 2.30.2 >
On Mon, Feb 14, 2022 at 3:45 PM Steve French <smfrench@gmail.com> wrote: > > Should I add: > cc:Stable # 5.12+ > > Thoughts? > > On Sun, Feb 13, 2022 at 4:41 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote: > > > > When we create a file with modefromsids we set an ACL that > > has one ACE for the magic modefromsid as well as a second ACE that > > grants full access to all authenticated users. > > > > When later we chante the mode on the file we strip away this, and other, > > ACE for authenticated users in set_chmod_dacl() and then just add back/update > > the modefromsid ACE. > > Thus leaving the file with a single ACE that is for the mode and no ACE > > to grant any user any rights to access the file. > > Fix this by always adding back also the modefromsid ACE so that we do not > > drop the rights to access the file. > > > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > > --- > > fs/cifs/cifsacl.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c > > index ee3aab3dd4ac..40cda87ce384 100644 > > --- a/fs/cifs/cifsacl.c > > +++ b/fs/cifs/cifsacl.c > > @@ -949,6 +949,9 @@ static void populate_new_aces(char *nacl_base, > > pnntace = (struct cifs_ace *) (nacl_base + nsize); > > nsize += setup_special_mode_ACE(pnntace, nmode); > > num_aces++; > > + pnntace = (struct cifs_ace *) (nacl_base + nsize); > > + nsize += setup_authusers_ACE(pnntace); > > + num_aces++; > > goto set_size; > > } > > > > @@ -1613,7 +1616,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode, > > nsecdesclen = secdesclen; > > if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */ > > if (mode_from_sid) > > - nsecdesclen += sizeof(struct cifs_ace); > > + nsecdesclen += 2 * sizeof(struct cifs_ace); > > else /* cifsacl */ > > nsecdesclen += 5 * sizeof(struct cifs_ace); > > } else { /* chown */ > > -- > > 2.30.2 > > > > > -- > Thanks, > > Steve Good catch. Changes look good. Please run the tests with memory sanitizers enabled for this one. Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index ee3aab3dd4ac..40cda87ce384 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -949,6 +949,9 @@ static void populate_new_aces(char *nacl_base, pnntace = (struct cifs_ace *) (nacl_base + nsize); nsize += setup_special_mode_ACE(pnntace, nmode); num_aces++; + pnntace = (struct cifs_ace *) (nacl_base + nsize); + nsize += setup_authusers_ACE(pnntace); + num_aces++; goto set_size; } @@ -1613,7 +1616,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode, nsecdesclen = secdesclen; if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */ if (mode_from_sid) - nsecdesclen += sizeof(struct cifs_ace); + nsecdesclen += 2 * sizeof(struct cifs_ace); else /* cifsacl */ nsecdesclen += 5 * sizeof(struct cifs_ace); } else { /* chown */
When we create a file with modefromsids we set an ACL that has one ACE for the magic modefromsid as well as a second ACE that grants full access to all authenticated users. When later we chante the mode on the file we strip away this, and other, ACE for authenticated users in set_chmod_dacl() and then just add back/update the modefromsid ACE. Thus leaving the file with a single ACE that is for the mode and no ACE to grant any user any rights to access the file. Fix this by always adding back also the modefromsid ACE so that we do not drop the rights to access the file. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/cifsacl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)