| Message ID | ZihxUuQOkZ6Zz363@neat (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [next] smb: client: Fix struct_group() usage in __packed structs | expand |
2024년 4월 24일 (수) 오전 11:41, Gustavo A. R. Silva <gustavoars@kernel.org>님이 작성: > > Use struct_group_attr() in __packed structs, instead of struct_group(). > > Below you can see the pahole output before/after changes: > > pahole -C smb2_file_network_open_info fs/smb/client/smb2ops.o > struct smb2_file_network_open_info { > union { > struct { > __le64 CreationTime; /* 0 8 */ > __le64 LastAccessTime; /* 8 8 */ > __le64 LastWriteTime; /* 16 8 */ > __le64 ChangeTime; /* 24 8 */ > __le64 AllocationSize; /* 32 8 */ > __le64 EndOfFile; /* 40 8 */ > __le32 Attributes; /* 48 4 */ > }; /* 0 56 */ > struct { > __le64 CreationTime; /* 0 8 */ > __le64 LastAccessTime; /* 8 8 */ > __le64 LastWriteTime; /* 16 8 */ > __le64 ChangeTime; /* 24 8 */ > __le64 AllocationSize; /* 32 8 */ > __le64 EndOfFile; /* 40 8 */ > __le32 Attributes; /* 48 4 */ > } network_open_info; /* 0 56 */ > }; /* 0 56 */ > __le32 Reserved; /* 56 4 */ > > /* size: 60, cachelines: 1, members: 2 */ > /* last cacheline: 60 bytes */ > } __attribute__((__packed__)); > > pahole -C smb2_file_network_open_info fs/smb/client/smb2ops.o > struct smb2_file_network_open_info { > union { > struct { > __le64 CreationTime; /* 0 8 */ > __le64 LastAccessTime; /* 8 8 */ > __le64 LastWriteTime; /* 16 8 */ > __le64 ChangeTime; /* 24 8 */ > __le64 AllocationSize; /* 32 8 */ > __le64 EndOfFile; /* 40 8 */ > __le32 Attributes; /* 48 4 */ > } __attribute__((__packed__)); /* 0 52 */ > struct { > __le64 CreationTime; /* 0 8 */ > __le64 LastAccessTime; /* 8 8 */ > __le64 LastWriteTime; /* 16 8 */ > __le64 ChangeTime; /* 24 8 */ > __le64 AllocationSize; /* 32 8 */ > __le64 EndOfFile; /* 40 8 */ > __le32 Attributes; /* 48 4 */ > } __attribute__((__packed__)) network_open_info; /* 0 52 */ > }; /* 0 52 */ > __le32 Reserved; /* 52 4 */ > > /* size: 56, cachelines: 1, members: 2 */ > /* last cacheline: 56 bytes */ > }; > > pahole -C smb_com_open_rsp fs/smb/client/cifssmb.o > struct smb_com_open_rsp { > ... > > union { > struct { > __le64 CreationTime; /* 48 8 */ > __le64 LastAccessTime; /* 56 8 */ > /* --- cacheline 1 boundary (64 bytes) --- */ > __le64 LastWriteTime; /* 64 8 */ > __le64 ChangeTime; /* 72 8 */ > __le32 FileAttributes; /* 80 4 */ > }; /* 48 40 */ > struct { > __le64 CreationTime; /* 48 8 */ > __le64 LastAccessTime; /* 56 8 */ > /* --- cacheline 1 boundary (64 bytes) --- */ > __le64 LastWriteTime; /* 64 8 */ > __le64 ChangeTime; /* 72 8 */ > __le32 FileAttributes; /* 80 4 */ > } common_attributes; /* 48 40 */ > }; /* 48 40 */ > > ... > > /* size: 111, cachelines: 2, members: 14 */ > /* last cacheline: 47 bytes */ > } __attribute__((__packed__)); > > pahole -C smb_com_open_rsp fs/smb/client/cifssmb.o > struct smb_com_open_rsp { > ... > > union { > struct { > __le64 CreationTime; /* 48 8 */ > __le64 LastAccessTime; /* 56 8 */ > /* --- cacheline 1 boundary (64 bytes) --- */ > __le64 LastWriteTime; /* 64 8 */ > __le64 ChangeTime; /* 72 8 */ > __le32 FileAttributes; /* 80 4 */ > } __attribute__((__packed__)); /* 48 36 */ > struct { > __le64 CreationTime; /* 48 8 */ > __le64 LastAccessTime; /* 56 8 */ > /* --- cacheline 1 boundary (64 bytes) --- */ > __le64 LastWriteTime; /* 64 8 */ > __le64 ChangeTime; /* 72 8 */ > __le32 FileAttributes; /* 80 4 */ > } __attribute__((__packed__)) common_attributes; /* 48 36 */ > }; /* 48 36 */ > > ... > > /* size: 107, cachelines: 2, members: 14 */ > /* last cacheline: 43 bytes */ > } __attribute__((__packed__)); > > pahole -C FILE_ALL_INFO fs/smb/client/cifssmb.o > typedef struct { > union { > struct { > __le64 CreationTime; /* 0 8 */ > __le64 LastAccessTime; /* 8 8 */ > __le64 LastWriteTime; /* 16 8 */ > __le64 ChangeTime; /* 24 8 */ > __le32 Attributes; /* 32 4 */ > }; /* 0 40 */ > struct { > __le64 CreationTime; /* 0 8 */ > __le64 LastAccessTime; /* 8 8 */ > __le64 LastWriteTime; /* 16 8 */ > __le64 ChangeTime; /* 24 8 */ > __le32 Attributes; /* 32 4 */ > } common_attributes; /* 0 40 */ > }; /* 0 40 */ > > ... > > /* size: 113, cachelines: 2, members: 17 */ > /* last cacheline: 49 bytes */ > } __attribute__((__packed__)) FILE_ALL_INFO; > > pahole -C FILE_ALL_INFO fs/smb/client/cifssmb.o > typedef struct { > union { > struct { > __le64 CreationTime; /* 0 8 */ > __le64 LastAccessTime; /* 8 8 */ > __le64 LastWriteTime; /* 16 8 */ > __le64 ChangeTime; /* 24 8 */ > __le32 Attributes; /* 32 4 */ > } __attribute__((__packed__)); /* 0 36 */ > struct { > __le64 CreationTime; /* 0 8 */ > __le64 LastAccessTime; /* 8 8 */ > __le64 LastWriteTime; /* 16 8 */ > __le64 ChangeTime; /* 24 8 */ > __le32 Attributes; /* 32 4 */ > } __attribute__((__packed__)) common_attributes; /* 0 36 */ > }; /* 0 36 */ > > ... > > /* size: 109, cachelines: 2, members: 17 */ > /* last cacheline: 45 bytes */ > } __attribute__((__packed__)) FILE_ALL_INFO; > > Fixes: 0015eb6e1238 ("smb: client, common: fix fortify warnings") > Cc: stable@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Looks good to me:) Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> Thanks!
Added to cifs-2.6.git for-next On Tue, Apr 23, 2024 at 11:36 PM Namjae Jeon <linkinjeon@kernel.org> wrote: > > 2024년 4월 24일 (수) 오전 11:41, Gustavo A. R. Silva <gustavoars@kernel.org>님이 작성: > > > > Use struct_group_attr() in __packed structs, instead of struct_group(). > > > > Below you can see the pahole output before/after changes: > > > > pahole -C smb2_file_network_open_info fs/smb/client/smb2ops.o > > struct smb2_file_network_open_info { > > union { > > struct { > > __le64 CreationTime; /* 0 8 */ > > __le64 LastAccessTime; /* 8 8 */ > > __le64 LastWriteTime; /* 16 8 */ > > __le64 ChangeTime; /* 24 8 */ > > __le64 AllocationSize; /* 32 8 */ > > __le64 EndOfFile; /* 40 8 */ > > __le32 Attributes; /* 48 4 */ > > }; /* 0 56 */ > > struct { > > __le64 CreationTime; /* 0 8 */ > > __le64 LastAccessTime; /* 8 8 */ > > __le64 LastWriteTime; /* 16 8 */ > > __le64 ChangeTime; /* 24 8 */ > > __le64 AllocationSize; /* 32 8 */ > > __le64 EndOfFile; /* 40 8 */ > > __le32 Attributes; /* 48 4 */ > > } network_open_info; /* 0 56 */ > > }; /* 0 56 */ > > __le32 Reserved; /* 56 4 */ > > > > /* size: 60, cachelines: 1, members: 2 */ > > /* last cacheline: 60 bytes */ > > } __attribute__((__packed__)); > > > > pahole -C smb2_file_network_open_info fs/smb/client/smb2ops.o > > struct smb2_file_network_open_info { > > union { > > struct { > > __le64 CreationTime; /* 0 8 */ > > __le64 LastAccessTime; /* 8 8 */ > > __le64 LastWriteTime; /* 16 8 */ > > __le64 ChangeTime; /* 24 8 */ > > __le64 AllocationSize; /* 32 8 */ > > __le64 EndOfFile; /* 40 8 */ > > __le32 Attributes; /* 48 4 */ > > } __attribute__((__packed__)); /* 0 52 */ > > struct { > > __le64 CreationTime; /* 0 8 */ > > __le64 LastAccessTime; /* 8 8 */ > > __le64 LastWriteTime; /* 16 8 */ > > __le64 ChangeTime; /* 24 8 */ > > __le64 AllocationSize; /* 32 8 */ > > __le64 EndOfFile; /* 40 8 */ > > __le32 Attributes; /* 48 4 */ > > } __attribute__((__packed__)) network_open_info; /* 0 52 */ > > }; /* 0 52 */ > > __le32 Reserved; /* 52 4 */ > > > > /* size: 56, cachelines: 1, members: 2 */ > > /* last cacheline: 56 bytes */ > > }; > > > > pahole -C smb_com_open_rsp fs/smb/client/cifssmb.o > > struct smb_com_open_rsp { > > ... > > > > union { > > struct { > > __le64 CreationTime; /* 48 8 */ > > __le64 LastAccessTime; /* 56 8 */ > > /* --- cacheline 1 boundary (64 bytes) --- */ > > __le64 LastWriteTime; /* 64 8 */ > > __le64 ChangeTime; /* 72 8 */ > > __le32 FileAttributes; /* 80 4 */ > > }; /* 48 40 */ > > struct { > > __le64 CreationTime; /* 48 8 */ > > __le64 LastAccessTime; /* 56 8 */ > > /* --- cacheline 1 boundary (64 bytes) --- */ > > __le64 LastWriteTime; /* 64 8 */ > > __le64 ChangeTime; /* 72 8 */ > > __le32 FileAttributes; /* 80 4 */ > > } common_attributes; /* 48 40 */ > > }; /* 48 40 */ > > > > ... > > > > /* size: 111, cachelines: 2, members: 14 */ > > /* last cacheline: 47 bytes */ > > } __attribute__((__packed__)); > > > > pahole -C smb_com_open_rsp fs/smb/client/cifssmb.o > > struct smb_com_open_rsp { > > ... > > > > union { > > struct { > > __le64 CreationTime; /* 48 8 */ > > __le64 LastAccessTime; /* 56 8 */ > > /* --- cacheline 1 boundary (64 bytes) --- */ > > __le64 LastWriteTime; /* 64 8 */ > > __le64 ChangeTime; /* 72 8 */ > > __le32 FileAttributes; /* 80 4 */ > > } __attribute__((__packed__)); /* 48 36 */ > > struct { > > __le64 CreationTime; /* 48 8 */ > > __le64 LastAccessTime; /* 56 8 */ > > /* --- cacheline 1 boundary (64 bytes) --- */ > > __le64 LastWriteTime; /* 64 8 */ > > __le64 ChangeTime; /* 72 8 */ > > __le32 FileAttributes; /* 80 4 */ > > } __attribute__((__packed__)) common_attributes; /* 48 36 */ > > }; /* 48 36 */ > > > > ... > > > > /* size: 107, cachelines: 2, members: 14 */ > > /* last cacheline: 43 bytes */ > > } __attribute__((__packed__)); > > > > pahole -C FILE_ALL_INFO fs/smb/client/cifssmb.o > > typedef struct { > > union { > > struct { > > __le64 CreationTime; /* 0 8 */ > > __le64 LastAccessTime; /* 8 8 */ > > __le64 LastWriteTime; /* 16 8 */ > > __le64 ChangeTime; /* 24 8 */ > > __le32 Attributes; /* 32 4 */ > > }; /* 0 40 */ > > struct { > > __le64 CreationTime; /* 0 8 */ > > __le64 LastAccessTime; /* 8 8 */ > > __le64 LastWriteTime; /* 16 8 */ > > __le64 ChangeTime; /* 24 8 */ > > __le32 Attributes; /* 32 4 */ > > } common_attributes; /* 0 40 */ > > }; /* 0 40 */ > > > > ... > > > > /* size: 113, cachelines: 2, members: 17 */ > > /* last cacheline: 49 bytes */ > > } __attribute__((__packed__)) FILE_ALL_INFO; > > > > pahole -C FILE_ALL_INFO fs/smb/client/cifssmb.o > > typedef struct { > > union { > > struct { > > __le64 CreationTime; /* 0 8 */ > > __le64 LastAccessTime; /* 8 8 */ > > __le64 LastWriteTime; /* 16 8 */ > > __le64 ChangeTime; /* 24 8 */ > > __le32 Attributes; /* 32 4 */ > > } __attribute__((__packed__)); /* 0 36 */ > > struct { > > __le64 CreationTime; /* 0 8 */ > > __le64 LastAccessTime; /* 8 8 */ > > __le64 LastWriteTime; /* 16 8 */ > > __le64 ChangeTime; /* 24 8 */ > > __le32 Attributes; /* 32 4 */ > > } __attribute__((__packed__)) common_attributes; /* 0 36 */ > > }; /* 0 36 */ > > > > ... > > > > /* size: 109, cachelines: 2, members: 17 */ > > /* last > > cacheline: 45 bytes */ > > } __attribute__((__packed__)) FILE_ALL_INFO; > > > > Fixes: 0015eb6e1238 ("smb: client, common: fix fortify warnings") > > Cc: stable@vger.kernel.org > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Looks good to me:) > Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> > Thanks! >
diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h index c0513fbb8a59..c46d418c1c0c 100644 --- a/fs/smb/client/cifspdu.h +++ b/fs/smb/client/cifspdu.h @@ -882,7 +882,7 @@ typedef struct smb_com_open_rsp { __u8 OplockLevel; __u16 Fid; __le32 CreateAction; - struct_group(common_attributes, + struct_group_attr(common_attributes, __packed, __le64 CreationTime; __le64 LastAccessTime; __le64 LastWriteTime; @@ -2266,7 +2266,7 @@ typedef struct { /* QueryFileInfo/QueryPathinfo (also for SetPath/SetFile) data buffer formats */ /******************************************************************************/ typedef struct { /* data block encoding of response to level 263 QPathInfo */ - struct_group(common_attributes, + struct_group_attr(common_attributes, __packed, __le64 CreationTime; __le64 LastAccessTime; __le64 LastWriteTime; diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h index c72a3b2886b7..2fccf0d4f53d 100644 --- a/fs/smb/client/smb2pdu.h +++ b/fs/smb/client/smb2pdu.h @@ -320,7 +320,7 @@ struct smb2_file_reparse_point_info { } __packed; struct smb2_file_network_open_info { - struct_group(network_open_info, + struct_group_attr(network_open_info, __packed, __le64 CreationTime; __le64 LastAccessTime; __le64 LastWriteTime;
Use struct_group_attr() in __packed structs, instead of struct_group(). Below you can see the pahole output before/after changes: pahole -C smb2_file_network_open_info fs/smb/client/smb2ops.o struct smb2_file_network_open_info { union { struct { __le64 CreationTime; /* 0 8 */ __le64 LastAccessTime; /* 8 8 */ __le64 LastWriteTime; /* 16 8 */ __le64 ChangeTime; /* 24 8 */ __le64 AllocationSize; /* 32 8 */ __le64 EndOfFile; /* 40 8 */ __le32 Attributes; /* 48 4 */ }; /* 0 56 */ struct { __le64 CreationTime; /* 0 8 */ __le64 LastAccessTime; /* 8 8 */ __le64 LastWriteTime; /* 16 8 */ __le64 ChangeTime; /* 24 8 */ __le64 AllocationSize; /* 32 8 */ __le64 EndOfFile; /* 40 8 */ __le32 Attributes; /* 48 4 */ } network_open_info; /* 0 56 */ }; /* 0 56 */ __le32 Reserved; /* 56 4 */ /* size: 60, cachelines: 1, members: 2 */ /* last cacheline: 60 bytes */ } __attribute__((__packed__)); pahole -C smb2_file_network_open_info fs/smb/client/smb2ops.o struct smb2_file_network_open_info { union { struct { __le64 CreationTime; /* 0 8 */ __le64 LastAccessTime; /* 8 8 */ __le64 LastWriteTime; /* 16 8 */ __le64 ChangeTime; /* 24 8 */ __le64 AllocationSize; /* 32 8 */ __le64 EndOfFile; /* 40 8 */ __le32 Attributes; /* 48 4 */ } __attribute__((__packed__)); /* 0 52 */ struct { __le64 CreationTime; /* 0 8 */ __le64 LastAccessTime; /* 8 8 */ __le64 LastWriteTime; /* 16 8 */ __le64 ChangeTime; /* 24 8 */ __le64 AllocationSize; /* 32 8 */ __le64 EndOfFile; /* 40 8 */ __le32 Attributes; /* 48 4 */ } __attribute__((__packed__)) network_open_info; /* 0 52 */ }; /* 0 52 */ __le32 Reserved; /* 52 4 */ /* size: 56, cachelines: 1, members: 2 */ /* last cacheline: 56 bytes */ }; pahole -C smb_com_open_rsp fs/smb/client/cifssmb.o struct smb_com_open_rsp { ... union { struct { __le64 CreationTime; /* 48 8 */ __le64 LastAccessTime; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ __le64 LastWriteTime; /* 64 8 */ __le64 ChangeTime; /* 72 8 */ __le32 FileAttributes; /* 80 4 */ }; /* 48 40 */ struct { __le64 CreationTime; /* 48 8 */ __le64 LastAccessTime; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ __le64 LastWriteTime; /* 64 8 */ __le64 ChangeTime; /* 72 8 */ __le32 FileAttributes; /* 80 4 */ } common_attributes; /* 48 40 */ }; /* 48 40 */ ... /* size: 111, cachelines: 2, members: 14 */ /* last cacheline: 47 bytes */ } __attribute__((__packed__)); pahole -C smb_com_open_rsp fs/smb/client/cifssmb.o struct smb_com_open_rsp { ... union { struct { __le64 CreationTime; /* 48 8 */ __le64 LastAccessTime; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ __le64 LastWriteTime; /* 64 8 */ __le64 ChangeTime; /* 72 8 */ __le32 FileAttributes; /* 80 4 */ } __attribute__((__packed__)); /* 48 36 */ struct { __le64 CreationTime; /* 48 8 */ __le64 LastAccessTime; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ __le64 LastWriteTime; /* 64 8 */ __le64 ChangeTime; /* 72 8 */ __le32 FileAttributes; /* 80 4 */ } __attribute__((__packed__)) common_attributes; /* 48 36 */ }; /* 48 36 */ ... /* size: 107, cachelines: 2, members: 14 */ /* last cacheline: 43 bytes */ } __attribute__((__packed__)); pahole -C FILE_ALL_INFO fs/smb/client/cifssmb.o typedef struct { union { struct { __le64 CreationTime; /* 0 8 */ __le64 LastAccessTime; /* 8 8 */ __le64 LastWriteTime; /* 16 8 */ __le64 ChangeTime; /* 24 8 */ __le32 Attributes; /* 32 4 */ }; /* 0 40 */ struct { __le64 CreationTime; /* 0 8 */ __le64 LastAccessTime; /* 8 8 */ __le64 LastWriteTime; /* 16 8 */ __le64 ChangeTime; /* 24 8 */ __le32 Attributes; /* 32 4 */ } common_attributes; /* 0 40 */ }; /* 0 40 */ ... /* size: 113, cachelines: 2, members: 17 */ /* last cacheline: 49 bytes */ } __attribute__((__packed__)) FILE_ALL_INFO; pahole -C FILE_ALL_INFO fs/smb/client/cifssmb.o typedef struct { union { struct { __le64 CreationTime; /* 0 8 */ __le64 LastAccessTime; /* 8 8 */ __le64 LastWriteTime; /* 16 8 */ __le64 ChangeTime; /* 24 8 */ __le32 Attributes; /* 32 4 */ } __attribute__((__packed__)); /* 0 36 */ struct { __le64 CreationTime; /* 0 8 */ __le64 LastAccessTime; /* 8 8 */ __le64 LastWriteTime; /* 16 8 */ __le64 ChangeTime; /* 24 8 */ __le32 Attributes; /* 32 4 */ } __attribute__((__packed__)) common_attributes; /* 0 36 */ }; /* 0 36 */ ... /* size: 109, cachelines: 2, members: 17 */ /* last cacheline: 45 bytes */ } __attribute__((__packed__)) FILE_ALL_INFO; Fixes: 0015eb6e1238 ("smb: client, common: fix fortify warnings") Cc: stable@vger.kernel.org Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- fs/smb/client/cifspdu.h | 4 ++-- fs/smb/client/smb2pdu.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)