Message ID | 20220920132045.5055-1-linkinjeon@kernel.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/3] ksmbd: change security id to the one samba used for posix extension | expand |
On 9/20/2022 9:20 AM, Namjae Jeon wrote: > Samba set SIDOWNER and SIDUNIX_GROUP in create posix context and > set SIDUNIX_USER/GROUP in other sids for posix extension. > This patch change security id to the one samba used. > > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > fs/ksmbd/oplock.c | 6 +++--- > fs/ksmbd/smb2pdu.c | 4 ++-- > fs/ksmbd/smb2pdu.h | 4 ++-- > 3 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c > index 2e56dac1fa6e..c26f02086783 100644 > --- a/fs/ksmbd/oplock.c > +++ b/fs/ksmbd/oplock.c > @@ -1616,7 +1616,7 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp) > memset(buf, 0, sizeof(struct create_posix_rsp)); > buf->ccontext.DataOffset = cpu_to_le16(offsetof > (struct create_posix_rsp, nlink)); > - buf->ccontext.DataLength = cpu_to_le32(52); > + buf->ccontext.DataLength = cpu_to_le32(56); "56" is a weird thing to code here. Can it be expressed as an offsetof or some sort of sizeof, for clarity and robustness? > buf->ccontext.NameOffset = cpu_to_le16(offsetof > (struct create_posix_rsp, Name)); > buf->ccontext.NameLength = cpu_to_le16(POSIX_CTXT_DATA_LEN); > @@ -1642,9 +1642,9 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp) > buf->reparse_tag = cpu_to_le32(fp->volatile_id); > buf->mode = cpu_to_le32(inode->i_mode); > id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)), > - SIDNFS_USER, (struct smb_sid *)&buf->SidBuffer[0]); > + SIDOWNER, (struct smb_sid *)&buf->SidBuffer[0]); > id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)), > - SIDNFS_GROUP, (struct smb_sid *)&buf->SidBuffer[20]); > + SIDUNIX_GROUP, (struct smb_sid *)&buf->SidBuffer[28]); Same comment for "28". offsetof(2 shorts and a sid), right? > } > > /* > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index f33a04e9e458..bc6c7ce17ea8 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -3573,9 +3573,9 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level, > if (d_info->hide_dot_file && d_info->name[0] == '.') > posix_info->DosAttributes |= FILE_ATTRIBUTE_HIDDEN_LE; > id_to_sid(from_kuid_munged(&init_user_ns, ksmbd_kstat->kstat->uid), > - SIDNFS_USER, (struct smb_sid *)&posix_info->SidBuffer[0]); > + SIDUNIX_USER, (struct smb_sid *)&posix_info->SidBuffer[0]); > id_to_sid(from_kgid_munged(&init_user_ns, ksmbd_kstat->kstat->gid), > - SIDNFS_GROUP, (struct smb_sid *)&posix_info->SidBuffer[20]); > + SIDUNIX_GROUP, (struct smb_sid *)&posix_info->SidBuffer[16]); And for "16", although now I'm also confused why it's 4 *less* than before. > memcpy(posix_info->name, conv_name, conv_len); > posix_info->name_len = cpu_to_le32(conv_len); > posix_info->NextEntryOffset = cpu_to_le32(next_entry_offset); > diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h > index af455278d005..32c525bf790a 100644 > --- a/fs/ksmbd/smb2pdu.h > +++ b/fs/ksmbd/smb2pdu.h > @@ -158,7 +158,7 @@ struct create_posix_rsp { > __le32 nlink; > __le32 reparse_tag; > __le32 mode; > - u8 SidBuffer[40]; > + u8 SidBuffer[44]; > } __packed; > > struct smb2_buffer_desc_v1 { > @@ -439,7 +439,7 @@ struct smb2_posix_info { > __le32 HardLinks; > __le32 ReparseTag; > __le32 Mode; > - u8 SidBuffer[40]; > + u8 SidBuffer[32]; Ok, so it's one buffer, which contains 2 sids? Ick. > __le32 name_len; > u8 name[1]; > /*
2022-09-21 5:56 GMT+09:00, Tom Talpey <tom@talpey.com>: > On 9/20/2022 9:20 AM, Namjae Jeon wrote: >> Samba set SIDOWNER and SIDUNIX_GROUP in create posix context and >> set SIDUNIX_USER/GROUP in other sids for posix extension. >> This patch change security id to the one samba used. >> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> fs/ksmbd/oplock.c | 6 +++--- >> fs/ksmbd/smb2pdu.c | 4 ++-- >> fs/ksmbd/smb2pdu.h | 4 ++-- >> 3 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c >> index 2e56dac1fa6e..c26f02086783 100644 >> --- a/fs/ksmbd/oplock.c >> +++ b/fs/ksmbd/oplock.c >> @@ -1616,7 +1616,7 @@ void create_posix_rsp_buf(char *cc, struct >> ksmbd_file *fp) >> memset(buf, 0, sizeof(struct create_posix_rsp)); >> buf->ccontext.DataOffset = cpu_to_le16(offsetof >> (struct create_posix_rsp, nlink)); >> - buf->ccontext.DataLength = cpu_to_le32(52); >> + buf->ccontext.DataLength = cpu_to_le32(56); > > "56" is a weird thing to code here. Can it be expressed as an > offsetof or some sort of sizeof, for clarity and robustness? I'll add a comment above this code. > >> buf->ccontext.NameOffset = cpu_to_le16(offsetof >> (struct create_posix_rsp, Name)); >> buf->ccontext.NameLength = cpu_to_le16(POSIX_CTXT_DATA_LEN); >> @@ -1642,9 +1642,9 @@ void create_posix_rsp_buf(char *cc, struct >> ksmbd_file *fp) >> buf->reparse_tag = cpu_to_le32(fp->volatile_id); >> buf->mode = cpu_to_le32(inode->i_mode); >> id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)), >> - SIDNFS_USER, (struct smb_sid *)&buf->SidBuffer[0]); >> + SIDOWNER, (struct smb_sid *)&buf->SidBuffer[0]); >> id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)), >> - SIDNFS_GROUP, (struct smb_sid *)&buf->SidBuffer[20]); >> + SIDUNIX_GROUP, (struct smb_sid *)&buf->SidBuffer[28]); > > Same comment for "28". offsetof(2 shorts and a sid), right? Ditto. > >> } >> >> /* >> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c >> index f33a04e9e458..bc6c7ce17ea8 100644 >> --- a/fs/ksmbd/smb2pdu.c >> +++ b/fs/ksmbd/smb2pdu.c >> @@ -3573,9 +3573,9 @@ static int smb2_populate_readdir_entry(struct >> ksmbd_conn *conn, int info_level, >> if (d_info->hide_dot_file && d_info->name[0] == '.') >> posix_info->DosAttributes |= FILE_ATTRIBUTE_HIDDEN_LE; >> id_to_sid(from_kuid_munged(&init_user_ns, ksmbd_kstat->kstat->uid), >> - SIDNFS_USER, (struct smb_sid *)&posix_info->SidBuffer[0]); >> + SIDUNIX_USER, (struct smb_sid *)&posix_info->SidBuffer[0]); >> id_to_sid(from_kgid_munged(&init_user_ns, ksmbd_kstat->kstat->gid), >> - SIDNFS_GROUP, (struct smb_sid *)&posix_info->SidBuffer[20]); >> + SIDUNIX_GROUP, (struct smb_sid *)&posix_info->SidBuffer[16]); > > And for "16", although now I'm also confused why it's 4 *less* than > before. Ditto. > > >> memcpy(posix_info->name, conv_name, conv_len); >> posix_info->name_len = cpu_to_le32(conv_len); >> posix_info->NextEntryOffset = cpu_to_le32(next_entry_offset); >> diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h >> index af455278d005..32c525bf790a 100644 >> --- a/fs/ksmbd/smb2pdu.h >> +++ b/fs/ksmbd/smb2pdu.h >> @@ -158,7 +158,7 @@ struct create_posix_rsp { >> __le32 nlink; >> __le32 reparse_tag; >> __le32 mode; >> - u8 SidBuffer[40]; >> + u8 SidBuffer[44]; >> } __packed; >> >> struct smb2_buffer_desc_v1 { >> @@ -439,7 +439,7 @@ struct smb2_posix_info { >> __le32 HardLinks; >> __le32 ReparseTag; >> __le32 Mode; >> - u8 SidBuffer[40]; >> + u8 SidBuffer[32]; > > Ok, so it's one buffer, which contains 2 sids? Ick. Yes. Thanks for your review! > >> __le32 name_len; >> u8 name[1]; >> /* >
diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c index 2e56dac1fa6e..c26f02086783 100644 --- a/fs/ksmbd/oplock.c +++ b/fs/ksmbd/oplock.c @@ -1616,7 +1616,7 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp) memset(buf, 0, sizeof(struct create_posix_rsp)); buf->ccontext.DataOffset = cpu_to_le16(offsetof (struct create_posix_rsp, nlink)); - buf->ccontext.DataLength = cpu_to_le32(52); + buf->ccontext.DataLength = cpu_to_le32(56); buf->ccontext.NameOffset = cpu_to_le16(offsetof (struct create_posix_rsp, Name)); buf->ccontext.NameLength = cpu_to_le16(POSIX_CTXT_DATA_LEN); @@ -1642,9 +1642,9 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp) buf->reparse_tag = cpu_to_le32(fp->volatile_id); buf->mode = cpu_to_le32(inode->i_mode); id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)), - SIDNFS_USER, (struct smb_sid *)&buf->SidBuffer[0]); + SIDOWNER, (struct smb_sid *)&buf->SidBuffer[0]); id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)), - SIDNFS_GROUP, (struct smb_sid *)&buf->SidBuffer[20]); + SIDUNIX_GROUP, (struct smb_sid *)&buf->SidBuffer[28]); } /* diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index f33a04e9e458..bc6c7ce17ea8 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -3573,9 +3573,9 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level, if (d_info->hide_dot_file && d_info->name[0] == '.') posix_info->DosAttributes |= FILE_ATTRIBUTE_HIDDEN_LE; id_to_sid(from_kuid_munged(&init_user_ns, ksmbd_kstat->kstat->uid), - SIDNFS_USER, (struct smb_sid *)&posix_info->SidBuffer[0]); + SIDUNIX_USER, (struct smb_sid *)&posix_info->SidBuffer[0]); id_to_sid(from_kgid_munged(&init_user_ns, ksmbd_kstat->kstat->gid), - SIDNFS_GROUP, (struct smb_sid *)&posix_info->SidBuffer[20]); + SIDUNIX_GROUP, (struct smb_sid *)&posix_info->SidBuffer[16]); memcpy(posix_info->name, conv_name, conv_len); posix_info->name_len = cpu_to_le32(conv_len); posix_info->NextEntryOffset = cpu_to_le32(next_entry_offset); diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h index af455278d005..32c525bf790a 100644 --- a/fs/ksmbd/smb2pdu.h +++ b/fs/ksmbd/smb2pdu.h @@ -158,7 +158,7 @@ struct create_posix_rsp { __le32 nlink; __le32 reparse_tag; __le32 mode; - u8 SidBuffer[40]; + u8 SidBuffer[44]; } __packed; struct smb2_buffer_desc_v1 { @@ -439,7 +439,7 @@ struct smb2_posix_info { __le32 HardLinks; __le32 ReparseTag; __le32 Mode; - u8 SidBuffer[40]; + u8 SidBuffer[32]; __le32 name_len; u8 name[1]; /*
Samba set SIDOWNER and SIDUNIX_GROUP in create posix context and set SIDUNIX_USER/GROUP in other sids for posix extension. This patch change security id to the one samba used. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- fs/ksmbd/oplock.c | 6 +++--- fs/ksmbd/smb2pdu.c | 4 ++-- fs/ksmbd/smb2pdu.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)