* [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension
@ 2022-09-20 13:20 Namjae Jeon
2022-09-20 13:20 ` [PATCH 2/3] ksmbd: set only file permisson to mode " Namjae Jeon
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Namjae Jeon @ 2022-09-20 13:20 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, senozhatsky, tom, atteh.mailbox, Namjae Jeon
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);
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];
/*
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] ksmbd: set only file permisson to mode for posix extension
2022-09-20 13:20 [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension Namjae Jeon
@ 2022-09-20 13:20 ` Namjae Jeon
2022-09-20 21:00 ` Tom Talpey
2022-09-20 13:20 ` [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response Namjae Jeon
2022-09-20 20:56 ` [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension Tom Talpey
2 siblings, 1 reply; 10+ messages in thread
From: Namjae Jeon @ 2022-09-20 13:20 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, senozhatsky, tom, atteh.mailbox, Namjae Jeon
Set only file permisson to mode for posix extension like samba.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
fs/ksmbd/oplock.c | 2 +-
fs/ksmbd/smb2pdu.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
index c26f02086783..9bfd1ef6debd 100644
--- a/fs/ksmbd/oplock.c
+++ b/fs/ksmbd/oplock.c
@@ -1640,7 +1640,7 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp)
buf->nlink = cpu_to_le32(inode->i_nlink);
buf->reparse_tag = cpu_to_le32(fp->volatile_id);
- buf->mode = cpu_to_le32(inode->i_mode);
+ buf->mode = cpu_to_le32(inode->i_mode & 0777);
id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)),
SIDOWNER, (struct smb_sid *)&buf->SidBuffer[0]);
id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)),
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index bc6c7ce17ea8..5c797cc09494 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -3565,7 +3565,7 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level,
posix_info->AllocationSize = cpu_to_le64(ksmbd_kstat->kstat->blocks << 9);
posix_info->DeviceId = cpu_to_le32(ksmbd_kstat->kstat->rdev);
posix_info->HardLinks = cpu_to_le32(ksmbd_kstat->kstat->nlink);
- posix_info->Mode = cpu_to_le32(ksmbd_kstat->kstat->mode);
+ posix_info->Mode = cpu_to_le32(ksmbd_kstat->kstat->mode & 0777);
posix_info->Inode = cpu_to_le64(ksmbd_kstat->kstat->ino);
posix_info->DosAttributes =
S_ISDIR(ksmbd_kstat->kstat->mode) ?
@@ -4732,7 +4732,7 @@ static int find_file_posix_info(struct smb2_query_info_rsp *rsp,
file_info->EndOfFile = cpu_to_le64(inode->i_size);
file_info->AllocationSize = cpu_to_le64(inode->i_blocks << 9);
file_info->HardLinks = cpu_to_le32(inode->i_nlink);
- file_info->Mode = cpu_to_le32(inode->i_mode);
+ file_info->Mode = cpu_to_le32(inode->i_mode & 0777);
file_info->DeviceId = cpu_to_le32(inode->i_rdev);
rsp->OutputBufferLength =
cpu_to_le32(sizeof(struct smb311_posix_qinfo));
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response
2022-09-20 13:20 [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension Namjae Jeon
2022-09-20 13:20 ` [PATCH 2/3] ksmbd: set only file permisson to mode " Namjae Jeon
@ 2022-09-20 13:20 ` Namjae Jeon
2022-09-20 21:05 ` Tom Talpey
2022-09-20 20:56 ` [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension Tom Talpey
2 siblings, 1 reply; 10+ messages in thread
From: Namjae Jeon @ 2022-09-20 13:20 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, senozhatsky, tom, atteh.mailbox, Namjae Jeon
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);
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:
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension
2022-09-20 13:20 [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension Namjae Jeon
2022-09-20 13:20 ` [PATCH 2/3] ksmbd: set only file permisson to mode " Namjae Jeon
2022-09-20 13:20 ` [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response Namjae Jeon
@ 2022-09-20 20:56 ` Tom Talpey
2022-09-20 22:00 ` Namjae Jeon
2 siblings, 1 reply; 10+ messages in thread
From: Tom Talpey @ 2022-09-20 20:56 UTC (permalink / raw)
To: Namjae Jeon, linux-cifs; +Cc: smfrench, senozhatsky, atteh.mailbox
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];
> /*
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] ksmbd: set only file permisson to mode for posix extension
2022-09-20 13:20 ` [PATCH 2/3] ksmbd: set only file permisson to mode " Namjae Jeon
@ 2022-09-20 21:00 ` Tom Talpey
0 siblings, 0 replies; 10+ messages in thread
From: Tom Talpey @ 2022-09-20 21:00 UTC (permalink / raw)
To: Namjae Jeon, linux-cifs; +Cc: smfrench, senozhatsky, atteh.mailbox
On 9/20/2022 9:20 AM, Namjae Jeon wrote:
> Set only file permisson to mode for posix extension like samba.
Suggest rewording this for clarity:
"Set file permission mode to match Samba server posix extension behavior".
Acked-by: Tom Talpey <tom@talpey.com>
>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
> fs/ksmbd/oplock.c | 2 +-
> fs/ksmbd/smb2pdu.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
> index c26f02086783..9bfd1ef6debd 100644
> --- a/fs/ksmbd/oplock.c
> +++ b/fs/ksmbd/oplock.c
> @@ -1640,7 +1640,7 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp)
>
> buf->nlink = cpu_to_le32(inode->i_nlink);
> buf->reparse_tag = cpu_to_le32(fp->volatile_id);
> - buf->mode = cpu_to_le32(inode->i_mode);
> + buf->mode = cpu_to_le32(inode->i_mode & 0777);
> id_to_sid(from_kuid_munged(&init_user_ns, vfsuid_into_kuid(vfsuid)),
> SIDOWNER, (struct smb_sid *)&buf->SidBuffer[0]);
> id_to_sid(from_kgid_munged(&init_user_ns, vfsgid_into_kgid(vfsgid)),
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index bc6c7ce17ea8..5c797cc09494 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -3565,7 +3565,7 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level,
> posix_info->AllocationSize = cpu_to_le64(ksmbd_kstat->kstat->blocks << 9);
> posix_info->DeviceId = cpu_to_le32(ksmbd_kstat->kstat->rdev);
> posix_info->HardLinks = cpu_to_le32(ksmbd_kstat->kstat->nlink);
> - posix_info->Mode = cpu_to_le32(ksmbd_kstat->kstat->mode);
> + posix_info->Mode = cpu_to_le32(ksmbd_kstat->kstat->mode & 0777);
> posix_info->Inode = cpu_to_le64(ksmbd_kstat->kstat->ino);
> posix_info->DosAttributes =
> S_ISDIR(ksmbd_kstat->kstat->mode) ?
> @@ -4732,7 +4732,7 @@ static int find_file_posix_info(struct smb2_query_info_rsp *rsp,
> file_info->EndOfFile = cpu_to_le64(inode->i_size);
> file_info->AllocationSize = cpu_to_le64(inode->i_blocks << 9);
> file_info->HardLinks = cpu_to_le32(inode->i_nlink);
> - file_info->Mode = cpu_to_le32(inode->i_mode);
> + file_info->Mode = cpu_to_le32(inode->i_mode & 0777);
> file_info->DeviceId = cpu_to_le32(inode->i_rdev);
> rsp->OutputBufferLength =
> cpu_to_le32(sizeof(struct smb311_posix_qinfo));
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response
2022-09-20 13:20 ` [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response Namjae Jeon
@ 2022-09-20 21:05 ` Tom Talpey
2022-09-20 22:20 ` Namjae Jeon
0 siblings, 1 reply; 10+ messages in thread
From: Tom Talpey @ 2022-09-20 21:05 UTC (permalink / raw)
To: Namjae Jeon, linux-cifs; +Cc: smfrench, senozhatsky, atteh.mailbox
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:
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension
2022-09-20 20:56 ` [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension Tom Talpey
@ 2022-09-20 22:00 ` Namjae Jeon
0 siblings, 0 replies; 10+ messages in thread
From: Namjae Jeon @ 2022-09-20 22:00 UTC (permalink / raw)
To: Tom Talpey; +Cc: linux-cifs, smfrench, senozhatsky, atteh.mailbox
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];
>> /*
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response
2022-09-20 21:05 ` Tom Talpey
@ 2022-09-20 22:20 ` Namjae Jeon
2022-09-22 22:28 ` Tom Talpey
0 siblings, 1 reply; 10+ messages in thread
From: Namjae Jeon @ 2022-09-20 22:20 UTC (permalink / raw)
To: Tom Talpey; +Cc: linux-cifs, smfrench, senozhatsky, atteh.mailbox
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:
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response
2022-09-20 22:20 ` Namjae Jeon
@ 2022-09-22 22:28 ` Tom Talpey
2022-09-22 23:37 ` Namjae Jeon
0 siblings, 1 reply; 10+ messages in thread
From: Tom Talpey @ 2022-09-22 22:28 UTC (permalink / raw)
To: Namjae Jeon; +Cc: linux-cifs, smfrench, senozhatsky, atteh.mailbox
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:
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response
2022-09-22 22:28 ` Tom Talpey
@ 2022-09-22 23:37 ` Namjae Jeon
0 siblings, 0 replies; 10+ messages in thread
From: Namjae Jeon @ 2022-09-22 23:37 UTC (permalink / raw)
To: Tom Talpey; +Cc: linux-cifs, smfrench, senozhatsky, atteh.mailbox
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:
>>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-09-22 23:38 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-20 13:20 [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension Namjae Jeon
2022-09-20 13:20 ` [PATCH 2/3] ksmbd: set only file permisson to mode " Namjae Jeon
2022-09-20 21:00 ` Tom Talpey
2022-09-20 13:20 ` [PATCH 3/3] ksmbd: fill sids in SMB_FIND_FILE_POSIX_INFO response Namjae Jeon
2022-09-20 21:05 ` Tom Talpey
2022-09-20 22:20 ` Namjae Jeon
2022-09-22 22:28 ` Tom Talpey
2022-09-22 23:37 ` Namjae Jeon
2022-09-20 20:56 ` [PATCH 1/3] ksmbd: change security id to the one samba used for posix extension Tom Talpey
2022-09-20 22:00 ` Namjae Jeon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).