| Message ID | 20220920132045.5055-3-linkinjeon@kernel.org |
|---|---|
| State | New |
| 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: > This patch fill missing sids in SMB_FIND_FILE_POSIX_INFO response. > > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > fs/ksmbd/smb2pdu.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index 5c797cc09494..9dd6033bc4de 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -4717,6 +4717,9 @@ static int find_file_posix_info(struct smb2_query_info_rsp *rsp, > { > struct smb311_posix_qinfo *file_info; > struct inode *inode = file_inode(fp->filp); > + struct user_namespace *user_ns = file_mnt_user_ns(fp->filp); > + vfsuid_t vfsuid = i_uid_into_vfsuid(user_ns, inode); > + vfsgid_t vfsgid = i_gid_into_vfsgid(user_ns, inode); > u64 time; > > file_info = (struct smb311_posix_qinfo *)rsp->Buffer; > @@ -4734,9 +4737,15 @@ static int find_file_posix_info(struct smb2_query_info_rsp *rsp, > file_info->HardLinks = cpu_to_le32(inode->i_nlink); > file_info->Mode = cpu_to_le32(inode->i_mode & 0777); > file_info->DeviceId = cpu_to_le32(inode->i_rdev); > + > + id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)), > + SIDUNIX_USER, (struct smb_sid *)&file_info->Sids[0]); > + id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)), > + SIDUNIX_GROUP, (struct smb_sid *)&file_info->Sids[16]); > + > rsp->OutputBufferLength = > - cpu_to_le32(sizeof(struct smb311_posix_qinfo)); > - inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo)); > + cpu_to_le32(sizeof(struct smb311_posix_qinfo) + 32); > + inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo) + 32); These 32's, and the one just below, are really sizeof(sidbuffer), right? Why code it as a raw number? Tom. > return 0; > } > > @@ -4858,7 +4867,7 @@ static int smb2_get_info_file(struct ksmbd_work *work, > rc = -EOPNOTSUPP; > } else { > rc = find_file_posix_info(rsp, fp, work->response_buf); > - file_infoclass_size = sizeof(struct smb311_posix_qinfo); > + file_infoclass_size = sizeof(struct smb311_posix_qinfo) + 32; > } > break; > default:
2022-09-21 6:05 GMT+09:00, Tom Talpey <tom@talpey.com>: > On 9/20/2022 9:20 AM, Namjae Jeon wrote: >> This patch fill missing sids in SMB_FIND_FILE_POSIX_INFO response. >> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> fs/ksmbd/smb2pdu.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c >> index 5c797cc09494..9dd6033bc4de 100644 >> --- a/fs/ksmbd/smb2pdu.c >> +++ b/fs/ksmbd/smb2pdu.c >> @@ -4717,6 +4717,9 @@ static int find_file_posix_info(struct >> smb2_query_info_rsp *rsp, >> { >> struct smb311_posix_qinfo *file_info; >> struct inode *inode = file_inode(fp->filp); >> + struct user_namespace *user_ns = file_mnt_user_ns(fp->filp); >> + vfsuid_t vfsuid = i_uid_into_vfsuid(user_ns, inode); >> + vfsgid_t vfsgid = i_gid_into_vfsgid(user_ns, inode); >> u64 time; >> >> file_info = (struct smb311_posix_qinfo *)rsp->Buffer; >> @@ -4734,9 +4737,15 @@ static int find_file_posix_info(struct >> smb2_query_info_rsp *rsp, >> file_info->HardLinks = cpu_to_le32(inode->i_nlink); >> file_info->Mode = cpu_to_le32(inode->i_mode & 0777); >> file_info->DeviceId = cpu_to_le32(inode->i_rdev); >> + >> + id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)), >> + SIDUNIX_USER, (struct smb_sid *)&file_info->Sids[0]); >> + id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)), >> + SIDUNIX_GROUP, (struct smb_sid *)&file_info->Sids[16]); >> + >> rsp->OutputBufferLength = >> - cpu_to_le32(sizeof(struct smb311_posix_qinfo)); >> - inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo)); >> + cpu_to_le32(sizeof(struct smb311_posix_qinfo) + 32); >> + inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo) + 32); > > These 32's, and the one just below, are really sizeof(sidbuffer), right? Yes. > > Why code it as a raw number? Sids is declared as flexible-array members. > > Tom. > >> return 0; >> } >> >> @@ -4858,7 +4867,7 @@ static int smb2_get_info_file(struct ksmbd_work >> *work, >> rc = -EOPNOTSUPP; >> } else { >> rc = find_file_posix_info(rsp, fp, work->response_buf); >> - file_infoclass_size = sizeof(struct smb311_posix_qinfo); >> + file_infoclass_size = sizeof(struct smb311_posix_qinfo) + 32; >> } >> break; >> default: >
On 9/20/2022 6:20 PM, Namjae Jeon wrote: > 2022-09-21 6:05 GMT+09:00, Tom Talpey <tom@talpey.com>: >> On 9/20/2022 9:20 AM, Namjae Jeon wrote: >>> This patch fill missing sids in SMB_FIND_FILE_POSIX_INFO response. >>> >>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >>> --- >>> fs/ksmbd/smb2pdu.c | 15 ++++++++++++--- >>> 1 file changed, 12 insertions(+), 3 deletions(-) >>> >>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c >>> index 5c797cc09494..9dd6033bc4de 100644 >>> --- a/fs/ksmbd/smb2pdu.c >>> +++ b/fs/ksmbd/smb2pdu.c >>> @@ -4717,6 +4717,9 @@ static int find_file_posix_info(struct >>> smb2_query_info_rsp *rsp, >>> { >>> struct smb311_posix_qinfo *file_info; >>> struct inode *inode = file_inode(fp->filp); >>> + struct user_namespace *user_ns = file_mnt_user_ns(fp->filp); >>> + vfsuid_t vfsuid = i_uid_into_vfsuid(user_ns, inode); >>> + vfsgid_t vfsgid = i_gid_into_vfsgid(user_ns, inode); >>> u64 time; >>> >>> file_info = (struct smb311_posix_qinfo *)rsp->Buffer; >>> @@ -4734,9 +4737,15 @@ static int find_file_posix_info(struct >>> smb2_query_info_rsp *rsp, >>> file_info->HardLinks = cpu_to_le32(inode->i_nlink); >>> file_info->Mode = cpu_to_le32(inode->i_mode & 0777); >>> file_info->DeviceId = cpu_to_le32(inode->i_rdev); >>> + >>> + id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)), >>> + SIDUNIX_USER, (struct smb_sid *)&file_info->Sids[0]); >>> + id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)), >>> + SIDUNIX_GROUP, (struct smb_sid *)&file_info->Sids[16]); >>> + >>> rsp->OutputBufferLength = >>> - cpu_to_le32(sizeof(struct smb311_posix_qinfo)); >>> - inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo)); >>> + cpu_to_le32(sizeof(struct smb311_posix_qinfo) + 32); >>> + inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo) + 32); >> >> These 32's, and the one just below, are really sizeof(sidbuffer), right? > Yes. >> >> Why code it as a raw number? > Sids is declared as flexible-array members. Ugh - worse than that. The smb311_posix_qinfo looks to have even more undefined payload: u8 Sids[]; /* * var sized owner SID * var sized group SID * le32 filenamelength * u8 filename[] */ This is pre-existing, nothing your patch should address, but does need attention before we attempt to standardize it!! MHO anyway. Tom. >> >> Tom. >> >>> return 0; >>> } >>> >>> @@ -4858,7 +4867,7 @@ static int smb2_get_info_file(struct ksmbd_work >>> *work, >>> rc = -EOPNOTSUPP; >>> } else { >>> rc = find_file_posix_info(rsp, fp, work->response_buf); >>> - file_infoclass_size = sizeof(struct smb311_posix_qinfo); >>> + file_infoclass_size = sizeof(struct smb311_posix_qinfo) + 32; >>> } >>> break; >>> default: >> >
2022-09-23 7:28 GMT+09:00, Tom Talpey <tom@talpey.com>: > On 9/20/2022 6:20 PM, Namjae Jeon wrote: >> 2022-09-21 6:05 GMT+09:00, Tom Talpey <tom@talpey.com>: >>> On 9/20/2022 9:20 AM, Namjae Jeon wrote: >>>> This patch fill missing sids in SMB_FIND_FILE_POSIX_INFO response. >>>> >>>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >>>> --- >>>> fs/ksmbd/smb2pdu.c | 15 ++++++++++++--- >>>> 1 file changed, 12 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c >>>> index 5c797cc09494..9dd6033bc4de 100644 >>>> --- a/fs/ksmbd/smb2pdu.c >>>> +++ b/fs/ksmbd/smb2pdu.c >>>> @@ -4717,6 +4717,9 @@ static int find_file_posix_info(struct >>>> smb2_query_info_rsp *rsp, >>>> { >>>> struct smb311_posix_qinfo *file_info; >>>> struct inode *inode = file_inode(fp->filp); >>>> + struct user_namespace *user_ns = file_mnt_user_ns(fp->filp); >>>> + vfsuid_t vfsuid = i_uid_into_vfsuid(user_ns, inode); >>>> + vfsgid_t vfsgid = i_gid_into_vfsgid(user_ns, inode); >>>> u64 time; >>>> >>>> file_info = (struct smb311_posix_qinfo *)rsp->Buffer; >>>> @@ -4734,9 +4737,15 @@ static int find_file_posix_info(struct >>>> smb2_query_info_rsp *rsp, >>>> file_info->HardLinks = cpu_to_le32(inode->i_nlink); >>>> file_info->Mode = cpu_to_le32(inode->i_mode & 0777); >>>> file_info->DeviceId = cpu_to_le32(inode->i_rdev); >>>> + >>>> + id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)), >>>> + SIDUNIX_USER, (struct smb_sid *)&file_info->Sids[0]); >>>> + id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)), >>>> + SIDUNIX_GROUP, (struct smb_sid *)&file_info->Sids[16]); >>>> + >>>> rsp->OutputBufferLength = >>>> - cpu_to_le32(sizeof(struct smb311_posix_qinfo)); >>>> - inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo)); >>>> + cpu_to_le32(sizeof(struct smb311_posix_qinfo) + 32); >>>> + inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo) + 32); >>> >>> These 32's, and the one just below, are really sizeof(sidbuffer), right? >> Yes. >>> >>> Why code it as a raw number? >> Sids is declared as flexible-array members. > > Ugh - worse than that. The smb311_posix_qinfo looks to have > even more undefined payload: When I checked the behavior of samba, I figured out that samba does not return filename and filename_length. Only user/group sids... It seems that the comment is probably wrong, but there is no specification documentation and there is no way to check it except packet analysis. > > u8 Sids[]; > /* > * var sized owner SID > * var sized group SID > * le32 filenamelength > * u8 filename[] > */ > > This is pre-existing, nothing your patch should address, but > does need attention before we attempt to standardize it!! It can be more updated when standardization is completed, Anyway, currently I am trying to match the behavior of samba. Please check if v2 patch is a little better. Thanks! > > MHO anyway. > > Tom. > > >>> >>> Tom. >>> >>>> return 0; >>>> } >>>> >>>> @@ -4858,7 +4867,7 @@ static int smb2_get_info_file(struct ksmbd_work >>>> *work, >>>> rc = -EOPNOTSUPP; >>>> } else { >>>> rc = find_file_posix_info(rsp, fp, work->response_buf); >>>> - file_infoclass_size = sizeof(struct smb311_posix_qinfo); >>>> + file_infoclass_size = sizeof(struct smb311_posix_qinfo) + 32; >>>> } >>>> break; >>>> default: >>> >> >
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 5c797cc09494..9dd6033bc4de 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -4717,6 +4717,9 @@ static int find_file_posix_info(struct smb2_query_info_rsp *rsp, { struct smb311_posix_qinfo *file_info; struct inode *inode = file_inode(fp->filp); + struct user_namespace *user_ns = file_mnt_user_ns(fp->filp); + vfsuid_t vfsuid = i_uid_into_vfsuid(user_ns, inode); + vfsgid_t vfsgid = i_gid_into_vfsgid(user_ns, inode); u64 time; file_info = (struct smb311_posix_qinfo *)rsp->Buffer; @@ -4734,9 +4737,15 @@ static int find_file_posix_info(struct smb2_query_info_rsp *rsp, file_info->HardLinks = cpu_to_le32(inode->i_nlink); file_info->Mode = cpu_to_le32(inode->i_mode & 0777); file_info->DeviceId = cpu_to_le32(inode->i_rdev); + + id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)), + SIDUNIX_USER, (struct smb_sid *)&file_info->Sids[0]); + id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)), + SIDUNIX_GROUP, (struct smb_sid *)&file_info->Sids[16]); + rsp->OutputBufferLength = - cpu_to_le32(sizeof(struct smb311_posix_qinfo)); - inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo)); + cpu_to_le32(sizeof(struct smb311_posix_qinfo) + 32); + inc_rfc1001_len(rsp_org, sizeof(struct smb311_posix_qinfo) + 32); return 0; } @@ -4858,7 +4867,7 @@ static int smb2_get_info_file(struct ksmbd_work *work, rc = -EOPNOTSUPP; } else { rc = find_file_posix_info(rsp, fp, work->response_buf); - file_infoclass_size = sizeof(struct smb311_posix_qinfo); + file_infoclass_size = sizeof(struct smb311_posix_qinfo) + 32; } break; default:
This patch fill missing sids in SMB_FIND_FILE_POSIX_INFO response. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- fs/ksmbd/smb2pdu.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)