overflow: Add struct_size_t() helper
Subsystems (12)
PATCH 1/1 overflow: Add struct_size_t() helper
While struct_size() is normally used in situations where the structure type already has a pointer instance, there are places where no variable is available. In the past, this has been worked around by using a typed NULL first argument, but this is a bit ugly. Add a helper to do this, and replace the handful of instances of the code pattern with it.
Instances were found with this Coccinelle script:
@struct_size_t@
identifier STRUCT, MEMBER;
expression COUNT;
@@
- struct_size((struct STRUCT *)\(0\|NULL\),
+ struct_size_t(struct STRUCT,
MEMBER, COUNT)
Suggested-by Christoph Hellwig <[email protected]>
Cc Jesse Brandeburg <[email protected]>
Cc Tony Nguyen <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Eric Dumazet <[email protected]>
Cc Jakub Kicinski <[email protected]>
Cc Paolo Abeni <[email protected]>
Cc James Smart <[email protected]>
Cc Keith Busch <[email protected]>
Cc Jens Axboe <[email protected]>
Cc Sagi Grimberg <[email protected]>
Cc HighPoint Linux Team <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc "Martin K. Petersen" <[email protected]>
Cc Kashyap Desai <[email protected]>
Cc Sumit Saxena <[email protected]>
Cc Shivasharan S <[email protected]>
Cc Don Brace <[email protected]>
Cc "Darrick J. Wong" <[email protected]>
Cc Dave Chinner <[email protected]>
Cc Guo Xuenan <[email protected]>
Cc Gwan-gyeong Mun <[email protected]>
Cc Nick Desaulniers <[email protected]>
Cc Daniel Latypov <[email protected]>
Cc kernel test robot <[email protected]>
Signed-off-by Kees Cook <[email protected]>
---
Unless there are objections, I'll just take this via my tree...
--- drivers/net/ethernet/intel/ice/ice_ddp.h | 9 ++++----- drivers/nvme/host/fc.c | 8 ++++---- drivers/scsi/hptiop.c | 4 ++-- drivers/scsi/megaraid/megaraid_sas_base.c | 12 ++++++------ drivers/scsi/megaraid/megaraid_sas_fp.c | 6 +++--- drivers/scsi/smartpqi/smartpqi_init.c | 2 +- fs/xfs/libxfs/xfs_btree.h | 2 +- fs/xfs/scrub/btree.h | 2 +- include/linux/overflow.h | 18 +++++++++++++++++- lib/overflow_kunit.c | 2 +- 10 files changed, 40 insertions(+), 25 deletions(-)
drivers/net/ethernet/intel/ice/ice_ddp.h +4 -5
@@ -185,7 +185,7 @@ struct ice_buf_hdr {#define ICE_MAX_ENTRIES_IN_BUF (hd_sz, ent_sz) \ ((ICE_PKG_BUF_SIZE - \ - struct_size((struct ice_buf_hdr *)0, section_entry, 1) - (hd_sz)) / \ + struct_size_t(struct ice_buf_hdr, section_entry, 1) - (hd_sz)) / \ (ent_sz))/* ice package section IDs */
@@ -297,7 +297,7 @@ struct ice_label_section { };#define ICE_MAX_LABELS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_label_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_label_section, \ label, 1) - \ sizeof(struct ice_label), \ sizeof(struct ice_label))
@@ -352,7 +352,7 @@ struct ice_boost_tcam_section { };#define ICE_MAX_BST_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_boost_tcam_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_boost_tcam_section, \ tcam, 1) - \ sizeof(struct ice_boost_tcam_entry), \ sizeof(struct ice_boost_tcam_entry))
@@ -372,8 +372,7 @@ struct ice_marker_ptype_tcam_section { };#define ICE_MAX_MARKER_PTYPE_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF( \ - struct_size((struct ice_marker_ptype_tcam_section *)0, tcam, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_marker_ptype_tcam_section, tcam, \ 1) - \ sizeof(struct ice_marker_ptype_tcam_entry), \ sizeof(struct ice_marker_ptype_tcam_entry))
drivers/nvme/host/fc.c +4 -4
@@ -2917,8 +2917,8 @@ nvme_fc_create_io_queues(struct nvme_fc_ctrl *ctrl) ret =nvme_alloc_io_tag_set (&ctrl->ctrl, &ctrl->tag_set, &nvme_fc_mq_ops,1 , -struct_size ((struct nvme_fcp_op_w_sgl *)NULL , priv, - ctrl->lport->ops->fcprqst_priv_sz)); + struct_size_t(struct nvme_fcp_op_w_sgl, priv, + ctrl->lport->ops->fcprqst_priv_sz));if (ret)return ret;
@@ -3536,8 +3536,8 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts, ret =nvme_alloc_admin_tag_set (&ctrl->ctrl, &ctrl->admin_tag_set, &nvme_fc_admin_mq_ops, -struct_size ((struct nvme_fcp_op_w_sgl *)NULL , priv, - ctrl->lport->ops->fcprqst_priv_sz)); + struct_size_t(struct nvme_fcp_op_w_sgl, priv, + ctrl->lport->ops->fcprqst_priv_sz));if (ret) goto fail_ctrl;
drivers/scsi/hptiop.c +2 -2
@@ -1394,8 +1394,8 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id) host->cmd_per_lun =le32_to_cpu (iop_config.max_requests); host->max_cmd_len =16 ; - req_size =struct_size ((struct hpt_iop_request_scsi_command *)0 , - sg_list, hba->max_sg_descriptors); + req_size = struct_size_t(struct hpt_iop_request_scsi_command, + sg_list, hba->max_sg_descriptors);if ((req_size &0x1f ) !=0 ) req_size = (req_size +0x1f ) & ~0x1f ;
drivers/scsi/megaraid/megaraid_sas_base.c +6 -6
@@ -5153,8 +5153,8 @@ static void megasas_update_ext_vd_details(struct megasas_instance *instance) fusion->max_map_sz = ventura_map_sz; }else { fusion->old_map_sz = -struct_size ((struct MR_FW_RAID_MAP *)0 , ldSpanMap, - instance->fw_supported_vd_count); + struct_size_t(struct MR_FW_RAID_MAP, ldSpanMap, + instance->fw_supported_vd_count); fusion->new_map_sz =sizeof (struct MR_FW_RAID_MAP_EXT ); fusion->max_map_sz =
@@ -5789,8 +5789,8 @@ megasas_setup_jbod_map(struct megasas_instance *instance)struct fusion_context *fusion = instance->ctrl_context;size_t pd_seq_map_sz; - pd_seq_map_sz =struct_size ((struct MR_PD_CFG_SEQ_NUM_SYNC *)0 , seq, -MAX_PHYSICAL_DEVICES ); + pd_seq_map_sz = struct_size_t(struct MR_PD_CFG_SEQ_NUM_SYNC, seq, +MAX_PHYSICAL_DEVICES ); instance->use_seqnum_jbod_fp = instance->support_seqnum_jbod_fp;
@@ -8033,8 +8033,8 @@ static void megasas_detach_one(struct pci_dev *pdev)if (instance->adapter_type !=MFI_SERIES ) {megasas_release_fusion (instance); pd_seq_map_sz = -struct_size ((struct MR_PD_CFG_SEQ_NUM_SYNC *)0 , - seq,MAX_PHYSICAL_DEVICES ); + struct_size_t(struct MR_PD_CFG_SEQ_NUM_SYNC, + seq,MAX_PHYSICAL_DEVICES );for (i =0 ; i <2 ; i++) {if (fusion->ld_map[i]) dma_free_coherent(&instance->pdev->dev,
drivers/scsi/megaraid/megaraid_sas_fp.c +3 -3
@@ -326,9 +326,9 @@ u8 MR_ValidateMapInfo(struct megasas_instance *instance, u64 map_id)else if (instance->supportmax256vd) expected_size =sizeof (struct MR_FW_RAID_MAP_EXT );else - expected_size =struct_size ((struct MR_FW_RAID_MAP *)0 , - ldSpanMap, -le16_to_cpu (pDrvRaidMap->ldCount)); + expected_size = struct_size_t(struct MR_FW_RAID_MAP, + ldSpanMap, +le16_to_cpu (pDrvRaidMap->ldCount));if (le32_to_cpu (pDrvRaidMap->totalSize) != expected_size) { dev_dbg(&instance->pdev->dev, "megasas: map info structure size 0x%x",
drivers/scsi/smartpqi/smartpqi_init.c +1 -1
@@ -5015,7 +5015,7 @@ static int pqi_create_queues(struct pqi_ctrl_info *ctrl_info) }#define PQI_REPORT_EVENT_CONFIG_BUFFER_LENGTH \ - struct_size((struct pqi_event_config *)0, descriptors, PQI_MAX_EVENT_DESCRIPTORS) + struct_size_t(struct pqi_event_config, descriptors, PQI_MAX_EVENT_DESCRIPTORS)static int pqi_configure_events (struct pqi_ctrl_info *ctrl_info,bool enable_events)
fs/xfs/libxfs/xfs_btree.h +1 -1
@@ -301,7 +301,7 @@ struct xfs_btree_curstatic inline size_t xfs_btree_cur_sizeof (unsigned int nlevels) { -return struct_size ((struct xfs_btree_cur *)NULL , bc_levels, nlevels); + return struct_size_t(struct xfs_btree_cur, bc_levels, nlevels); }/* cursor flags */
fs/xfs/scrub/btree.h +1 -1
@@ -60,7 +60,7 @@ struct xchk_btree {static inline size_t xchk_btree_sizeof (unsigned int nlevels) { -return struct_size ((struct xchk_btree *)NULL , lastkey, nlevels -1 ); + return struct_size_t(struct xchk_btree, lastkey, nlevels - 1); } int xchk_btree(struct xfs_scrub *sc, struct xfs_btree_cur *cur,
include/linux/overflow.h +17 -1
@@ -283,7 +283,7 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* @member: Name of the array member. * @count: Number of elements in the array. * -* Calculates size of memory needed for structure @p followed by an +* Calculates size of memory needed for structure of @p followed by an * array of @count number of @member elements. * * Return: number of bytes needed or SIZE_MAX on overflow.
@@ -293,4 +293,20 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) sizeof(*(p)) + flex_array_size(p, member, count), \ size_add(sizeof(*(p)), flex_array_size(p, member, count))) +/** +* struct_size_t() - Calculate size of structure with trailing flexible array +* @type: structure type name. +* @member: Name of the array member. +* @count: Number of elements in the array. +* +* Calculates size of memory needed for structure @type followed by an +* array of @count number of @member elements. Prefer using struct_size() +* when possible instead, to keep calculations associated with a specific +* instance variable of type @type. +* +* Return: number of bytes needed or SIZE_MAX on overflow. +*/ +#define struct_size_t (type, member, count) \ + struct_size((type *)NULL, member, count) +#endif /* __LINUX_OVERFLOW_H */
lib/overflow_kunit.c +1 -1
@@ -649,7 +649,7 @@ struct __test_flex_array {static void overflow_size_helpers_test (struct kunit *test) {/* Make sure struct_size() can be used in a constant expression. */ -u8 ce_array[struct_size ((struct __test_flex_array *)0 , data,55 )]; + u8 ce_array[struct_size_t(struct __test_flex_array, data, 55)];struct __test_flex_array *obj;int count =0 ;int var;
Signature · 1 line
2.34.1
Quoted 220 lines · Kees Cook
While
struct_size()is normally used in situations where the structure type already has a pointer instance, there are places where no variable is available. In the past, this has been worked around by using a typed NULL first argument, but this is a bit ugly. Add a helper to do this, and replace the handful of instances of the code pattern with it.Instances were found with this Coccinelle script:
@struct_size_t@
identifier STRUCT, MEMBER;
expression COUNT;
@@(quoted diff) +1
-1- struct_size((struct STRUCT *)\(0\|NULL\), + struct_size_t(struct STRUCT, MEMBER, COUNT)Suggested-by Christoph Hellwig <[email protected]>Cc Jesse Brandeburg <[email protected]>Cc Tony Nguyen <[email protected]>Cc "David S. Miller" <[email protected]>Cc Eric Dumazet <[email protected]>Cc Jakub Kicinski <[email protected]>Cc Paolo Abeni <[email protected]>Cc James Smart <[email protected]>Cc Keith Busch <[email protected]>Cc Jens Axboe <[email protected]>Cc Sagi Grimberg <[email protected]>Cc HighPoint Linux Team <[email protected]>Cc "James E.J. Bottomley" <[email protected]>Cc "Martin K. Petersen" <[email protected]>Cc Kashyap Desai <[email protected]>Cc Sumit Saxena <[email protected]>Cc Shivasharan S <[email protected]>Cc Don Brace <[email protected]>Cc "Darrick J. Wong" <[email protected]>Cc Dave Chinner <[email protected]>Cc Guo Xuenan <[email protected]>Cc Gwan-gyeong Mun <[email protected]>Cc Nick Desaulniers <[email protected]>Cc Daniel Latypov <[email protected]>Cc kernel test robot <[email protected]>Signed-off-by Kees Cook <[email protected]>---
Unless there are objections, I'll just take this via my tree...--- drivers/net/ethernet/intel/ice/ice_ddp.h | 9 ++++----- drivers/nvme/host/fc.c | 8 ++++---- drivers/scsi/hptiop.c | 4 ++-- drivers/scsi/megaraid/megaraid_sas_base.c | 12 ++++++------ drivers/scsi/megaraid/megaraid_sas_fp.c | 6 +++--- drivers/scsi/smartpqi/smartpqi_init.c | 2 +- fs/xfs/libxfs/xfs_btree.h | 2 +- fs/xfs/scrub/btree.h | 2 +- include/linux/overflow.h | 18 +++++++++++++++++- lib/overflow_kunit.c | 2 +- 10 files changed, 40 insertions(+), 25 deletions(-)drivers/net/ethernet/intel/ice/ice_ddp.h +4
-5@@ -185,7 +185,7 @@ struct ice_buf_hdr {#define ICE_MAX_ENTRIES_IN_BUF (hd_sz, ent_sz) \ ((ICE_PKG_BUF_SIZE - \ - struct_size((struct ice_buf_hdr *)0, section_entry, 1) - (hd_sz)) / \ + struct_size_t(struct ice_buf_hdr, section_entry, 1) - (hd_sz)) / \ (ent_sz))/* ice package section IDs */ @@ -297,7 +297,7 @@ struct ice_label_section { };#define ICE_MAX_LABELS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_label_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_label_section, \ label, 1) - \ sizeof(struct ice_label), \ sizeof(struct ice_label))@@ -352,7 +352,7 @@ struct ice_boost_tcam_section { };#define ICE_MAX_BST_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_boost_tcam_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_boost_tcam_section, \ tcam, 1) - \ sizeof(struct ice_boost_tcam_entry), \ sizeof(struct ice_boost_tcam_entry))@@ -372,8 +372,7 @@ struct ice_marker_ptype_tcam_section { };#define ICE_MAX_MARKER_PTYPE_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF( \ - struct_size((struct ice_marker_ptype_tcam_section *)0, tcam, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_marker_ptype_tcam_section, tcam, \ 1) - \ sizeof(struct ice_marker_ptype_tcam_entry), \ sizeof(struct ice_marker_ptype_tcam_entry))drivers/nvme/host/fc.c +4
-4@@ -2917,8 +2917,8 @@ nvme_fc_create_io_queues(struct nvme_fc_ctrl *ctrl) ret =nvme_alloc_io_tag_set (&ctrl->ctrl, &ctrl->tag_set, &nvme_fc_mq_ops,1 , -struct_size ((struct nvme_fcp_op_w_sgl *)NULL , priv, - ctrl->lport->ops->fcprqst_priv_sz)); + struct_size_t(struct nvme_fcp_op_w_sgl, priv, + ctrl->lport->ops->fcprqst_priv_sz));if (ret)return ret;@@ -3536,8 +3536,8 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts, ret =nvme_alloc_admin_tag_set (&ctrl->ctrl, &ctrl->admin_tag_set, &nvme_fc_admin_mq_ops, -struct_size ((struct nvme_fcp_op_w_sgl *)NULL , priv, - ctrl->lport->ops->fcprqst_priv_sz)); + struct_size_t(struct nvme_fcp_op_w_sgl, priv, + ctrl->lport->ops->fcprqst_priv_sz));if (ret) goto fail_ctrl;drivers/scsi/hptiop.c +2
-2@@ -1394,8 +1394,8 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id) host->cmd_per_lun =le32_to_cpu (iop_config.max_requests); host->max_cmd_len =16 ; - req_size =struct_size ((struct hpt_iop_request_scsi_command *)0 , - sg_list, hba->max_sg_descriptors); + req_size = struct_size_t(struct hpt_iop_request_scsi_command, + sg_list, hba->max_sg_descriptors);if ((req_size &0x1f ) !=0 ) req_size = (req_size +0x1f ) & ~0x1f ;drivers/scsi/megaraid/megaraid_sas_base.c +6
-6@@ -5153,8 +5153,8 @@ static void megasas_update_ext_vd_details(struct megasas_instance *instance) fusion->max_map_sz = ventura_map_sz; }else { fusion->old_map_sz = -struct_size ((struct MR_FW_RAID_MAP *)0 , ldSpanMap, - instance->fw_supported_vd_count); + struct_size_t(struct MR_FW_RAID_MAP, ldSpanMap, + instance->fw_supported_vd_count); fusion->new_map_sz =sizeof (struct MR_FW_RAID_MAP_EXT ); fusion->max_map_sz =@@ -5789,8 +5789,8 @@ megasas_setup_jbod_map(struct megasas_instance *instance)struct fusion_context *fusion = instance->ctrl_context;size_t pd_seq_map_sz; - pd_seq_map_sz =struct_size ((struct MR_PD_CFG_SEQ_NUM_SYNC *)0 , seq, -MAX_PHYSICAL_DEVICES ); + pd_seq_map_sz = struct_size_t(struct MR_PD_CFG_SEQ_NUM_SYNC, seq, +MAX_PHYSICAL_DEVICES ); instance->use_seqnum_jbod_fp = instance->support_seqnum_jbod_fp;@@ -8033,8 +8033,8 @@ static void megasas_detach_one(struct pci_dev *pdev)if (instance->adapter_type !=MFI_SERIES ) {megasas_release_fusion (instance); pd_seq_map_sz = -struct_size ((struct MR_PD_CFG_SEQ_NUM_SYNC *)0 , - seq,MAX_PHYSICAL_DEVICES ); + struct_size_t(struct MR_PD_CFG_SEQ_NUM_SYNC, + seq,MAX_PHYSICAL_DEVICES );for (i =0 ; i <2 ; i++) {if (fusion->ld_map[i]) dma_free_coherent(&instance->pdev->dev,drivers/scsi/megaraid/megaraid_sas_fp.c +3
-3@@ -326,9 +326,9 @@ u8 MR_ValidateMapInfo(struct megasas_instance *instance, u64 map_id)else if (instance->supportmax256vd) expected_size =sizeof (struct MR_FW_RAID_MAP_EXT );else - expected_size =struct_size ((struct MR_FW_RAID_MAP *)0 , - ldSpanMap, -le16_to_cpu (pDrvRaidMap->ldCount)); + expected_size = struct_size_t(struct MR_FW_RAID_MAP, + ldSpanMap, +le16_to_cpu (pDrvRaidMap->ldCount));if (le32_to_cpu (pDrvRaidMap->totalSize) != expected_size) { dev_dbg(&instance->pdev->dev, "megasas: map info structure size 0x%x",drivers/scsi/smartpqi/smartpqi_init.c +1
-1@@ -5015,7 +5015,7 @@ static int pqi_create_queues(struct pqi_ctrl_info *ctrl_info) }#define PQI_REPORT_EVENT_CONFIG_BUFFER_LENGTH \ - struct_size((struct pqi_event_config *)0, descriptors, PQI_MAX_EVENT_DESCRIPTORS) + struct_size_t(struct pqi_event_config, descriptors, PQI_MAX_EVENT_DESCRIPTORS)static int pqi_configure_events (struct pqi_ctrl_info *ctrl_info,bool enable_events)fs/xfs/libxfs/xfs_btree.h +0
-0
fs/xfs/libxfs/xfs_btree.h struct xfs_btree_cur · line 304
Quoted 5 lines
fs/xfs/libxfs/xfs_btree.h +1
-1@@ -301,7 +301,7 @@ struct xfs_btree_curstatic inline size_t xfs_btree_cur_sizeof (unsigned int nlevels) { -return struct_size ((struct xfs_btree_cur *)NULL , bc_levels, nlevels); + return struct_size_t(struct xfs_btree_cur, bc_levels, nlevels);
Oh, hey, this thing ^^^^^^^^ again. I'm excited!
Reviewed-by Darrick J. Wong <[email protected]>
--D
Quoted 66 lines
fs/xfs/libxfs/xfs_btree.h +0
-0}/* cursor flags */ fs/xfs/scrub/btree.h +1
-1@@ -60,7 +60,7 @@ struct xchk_btree {static inline size_t xchk_btree_sizeof (unsigned int nlevels) { -return struct_size ((struct xchk_btree *)NULL , lastkey, nlevels -1 ); + return struct_size_t(struct xchk_btree, lastkey, nlevels - 1); } int xchk_btree(struct xfs_scrub *sc, struct xfs_btree_cur *cur,include/linux/overflow.h +17
-1@@ -283,7 +283,7 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* @member: Name of the array member. * @count: Number of elements in the array. * -* Calculates size of memory needed for structure @p followed by an +* Calculates size of memory needed for structure of @p followed by an * array of @count number of @member elements. * * Return: number of bytes needed or SIZE_MAX on overflow. @@ -293,4 +293,20 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) sizeof(*(p)) + flex_array_size(p, member, count), \ size_add(sizeof(*(p)), flex_array_size(p, member, count))) +/** +* struct_size_t() - Calculate size of structure with trailing flexible array +* @type: structure type name. +* @member: Name of the array member. +* @count: Number of elements in the array. +* +* Calculates size of memory needed for structure @type followed by an +* array of @count number of @member elements. Prefer using struct_size() +* when possible instead, to keep calculations associated with a specific +* instance variable of type @type. +* +* Return: number of bytes needed or SIZE_MAX on overflow. +*/ +#define struct_size_t (type, member, count) \ + struct_size((type *)NULL, member, count) +#endif /* __LINUX_OVERFLOW_H */ lib/overflow_kunit.c +1
-1@@ -649,7 +649,7 @@ struct __test_flex_array {static void overflow_size_helpers_test (struct kunit *test) {/* Make sure struct_size() can be used in a constant expression. */ -u8 ce_array[struct_size ((struct __test_flex_array *)0 , data,55 )]; + u8 ce_array[struct_size_t(struct __test_flex_array, data, 55)];struct __test_flex_array *obj;int count =0 ;int var;Signature · 1 line
2.34.1
Quoted 13 lines · Kees Cook
While
struct_size()is normally used in situations where the structure type already has a pointer instance, there are places where no variable is available. In the past, this has been worked around by using a typed NULL first argument, but this is a bit ugly. Add a helper to do this, and replace the handful of instances of the code pattern with it.Instances were found with this Coccinelle script:
@struct_size_t@
identifier STRUCT, MEMBER;
expression COUNT;
@@
Review
Quoted 3 lines
(quoted diff) +1
-1- struct_size((struct STRUCT *)\(0\|NULL\), + struct_size_t(struct STRUCT, MEMBER, COUNT)
This indeed was much needed.
Plus, we save 3 characters on each line. :D
Quoted 27 lines
Suggested-by Christoph Hellwig <[email protected]>Cc Jesse Brandeburg <[email protected]>Cc Tony Nguyen <[email protected]>Cc "David S. Miller" <[email protected]>Cc Eric Dumazet <[email protected]>Cc Jakub Kicinski <[email protected]>Cc Paolo Abeni <[email protected]>Cc James Smart <[email protected]>Cc Keith Busch <[email protected]>Cc Jens Axboe <[email protected]>Cc Sagi Grimberg <[email protected]>Cc HighPoint Linux Team <[email protected]>Cc "James E.J. Bottomley" <[email protected]>Cc "Martin K. Petersen" <[email protected]>Cc Kashyap Desai <[email protected]>Cc Sumit Saxena <[email protected]>Cc Shivasharan S <[email protected]>Cc Don Brace <[email protected]>Cc "Darrick J. Wong" <[email protected]>Cc Dave Chinner <[email protected]>Cc Guo Xuenan <[email protected]>Cc Gwan-gyeong Mun <[email protected]>Cc Nick Desaulniers <[email protected]>Cc Daniel Latypov <[email protected]>Cc kernel test robot <[email protected]>
Quoted 8 lines
Reviewed-by Gustavo A. R. Silva <[email protected]>
Thanks!
--
Gustavo
Quoted 244 lines
---
Unless there are objections, I'll just take this via my tree...--- drivers/net/ethernet/intel/ice/ice_ddp.h | 9 ++++----- drivers/nvme/host/fc.c | 8 ++++---- drivers/scsi/hptiop.c | 4 ++-- drivers/scsi/megaraid/megaraid_sas_base.c | 12 ++++++------ drivers/scsi/megaraid/megaraid_sas_fp.c | 6 +++--- drivers/scsi/smartpqi/smartpqi_init.c | 2 +- fs/xfs/libxfs/xfs_btree.h | 2 +- fs/xfs/scrub/btree.h | 2 +- include/linux/overflow.h | 18 +++++++++++++++++- lib/overflow_kunit.c | 2 +- 10 files changed, 40 insertions(+), 25 deletions(-)drivers/net/ethernet/intel/ice/ice_ddp.h +4
-5@@ -185,7 +185,7 @@ struct ice_buf_hdr {#define ICE_MAX_ENTRIES_IN_BUF (hd_sz, ent_sz) \ ((ICE_PKG_BUF_SIZE - \ - struct_size((struct ice_buf_hdr *)0, section_entry, 1) - (hd_sz)) / \ + struct_size_t(struct ice_buf_hdr, section_entry, 1) - (hd_sz)) / \ (ent_sz))/* ice package section IDs */ @@ -297,7 +297,7 @@ struct ice_label_section { };#define ICE_MAX_LABELS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_label_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_label_section, \ label, 1) - \ sizeof(struct ice_label), \ sizeof(struct ice_label))@@ -352,7 +352,7 @@ struct ice_boost_tcam_section { };#define ICE_MAX_BST_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_boost_tcam_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_boost_tcam_section, \ tcam, 1) - \ sizeof(struct ice_boost_tcam_entry), \ sizeof(struct ice_boost_tcam_entry))@@ -372,8 +372,7 @@ struct ice_marker_ptype_tcam_section { };#define ICE_MAX_MARKER_PTYPE_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF( \ - struct_size((struct ice_marker_ptype_tcam_section *)0, tcam, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_marker_ptype_tcam_section, tcam, \ 1) - \ sizeof(struct ice_marker_ptype_tcam_entry), \ sizeof(struct ice_marker_ptype_tcam_entry))drivers/nvme/host/fc.c +4
-4@@ -2917,8 +2917,8 @@ nvme_fc_create_io_queues(struct nvme_fc_ctrl *ctrl) ret =nvme_alloc_io_tag_set (&ctrl->ctrl, &ctrl->tag_set, &nvme_fc_mq_ops,1 , -struct_size ((struct nvme_fcp_op_w_sgl *)NULL , priv, - ctrl->lport->ops->fcprqst_priv_sz)); + struct_size_t(struct nvme_fcp_op_w_sgl, priv, + ctrl->lport->ops->fcprqst_priv_sz));if (ret)return ret;@@ -3536,8 +3536,8 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts, ret =nvme_alloc_admin_tag_set (&ctrl->ctrl, &ctrl->admin_tag_set, &nvme_fc_admin_mq_ops, -struct_size ((struct nvme_fcp_op_w_sgl *)NULL , priv, - ctrl->lport->ops->fcprqst_priv_sz)); + struct_size_t(struct nvme_fcp_op_w_sgl, priv, + ctrl->lport->ops->fcprqst_priv_sz));if (ret) goto fail_ctrl;drivers/scsi/hptiop.c +2
-2@@ -1394,8 +1394,8 @@ static int hptiop_probe(struct pci_dev *pcidev, const struct pci_device_id *id) host->cmd_per_lun =le32_to_cpu (iop_config.max_requests); host->max_cmd_len =16 ; - req_size =struct_size ((struct hpt_iop_request_scsi_command *)0 , - sg_list, hba->max_sg_descriptors); + req_size = struct_size_t(struct hpt_iop_request_scsi_command, + sg_list, hba->max_sg_descriptors);if ((req_size &0x1f ) !=0 ) req_size = (req_size +0x1f ) & ~0x1f ;drivers/scsi/megaraid/megaraid_sas_base.c +6
-6@@ -5153,8 +5153,8 @@ static void megasas_update_ext_vd_details(struct megasas_instance *instance) fusion->max_map_sz = ventura_map_sz; }else { fusion->old_map_sz = -struct_size ((struct MR_FW_RAID_MAP *)0 , ldSpanMap, - instance->fw_supported_vd_count); + struct_size_t(struct MR_FW_RAID_MAP, ldSpanMap, + instance->fw_supported_vd_count); fusion->new_map_sz =sizeof (struct MR_FW_RAID_MAP_EXT ); fusion->max_map_sz =@@ -5789,8 +5789,8 @@ megasas_setup_jbod_map(struct megasas_instance *instance)struct fusion_context *fusion = instance->ctrl_context;size_t pd_seq_map_sz; - pd_seq_map_sz =struct_size ((struct MR_PD_CFG_SEQ_NUM_SYNC *)0 , seq, -MAX_PHYSICAL_DEVICES ); + pd_seq_map_sz = struct_size_t(struct MR_PD_CFG_SEQ_NUM_SYNC, seq, +MAX_PHYSICAL_DEVICES ); instance->use_seqnum_jbod_fp = instance->support_seqnum_jbod_fp;@@ -8033,8 +8033,8 @@ static void megasas_detach_one(struct pci_dev *pdev)if (instance->adapter_type !=MFI_SERIES ) {megasas_release_fusion (instance); pd_seq_map_sz = -struct_size ((struct MR_PD_CFG_SEQ_NUM_SYNC *)0 , - seq,MAX_PHYSICAL_DEVICES ); + struct_size_t(struct MR_PD_CFG_SEQ_NUM_SYNC, + seq,MAX_PHYSICAL_DEVICES );for (i =0 ; i <2 ; i++) {if (fusion->ld_map[i]) dma_free_coherent(&instance->pdev->dev,drivers/scsi/megaraid/megaraid_sas_fp.c +3
-3@@ -326,9 +326,9 @@ u8 MR_ValidateMapInfo(struct megasas_instance *instance, u64 map_id)else if (instance->supportmax256vd) expected_size =sizeof (struct MR_FW_RAID_MAP_EXT );else - expected_size =struct_size ((struct MR_FW_RAID_MAP *)0 , - ldSpanMap, -le16_to_cpu (pDrvRaidMap->ldCount)); + expected_size = struct_size_t(struct MR_FW_RAID_MAP, + ldSpanMap, +le16_to_cpu (pDrvRaidMap->ldCount));if (le32_to_cpu (pDrvRaidMap->totalSize) != expected_size) { dev_dbg(&instance->pdev->dev, "megasas: map info structure size 0x%x",drivers/scsi/smartpqi/smartpqi_init.c +1
-1@@ -5015,7 +5015,7 @@ static int pqi_create_queues(struct pqi_ctrl_info *ctrl_info) }#define PQI_REPORT_EVENT_CONFIG_BUFFER_LENGTH \ - struct_size((struct pqi_event_config *)0, descriptors, PQI_MAX_EVENT_DESCRIPTORS) + struct_size_t(struct pqi_event_config, descriptors, PQI_MAX_EVENT_DESCRIPTORS)static int pqi_configure_events (struct pqi_ctrl_info *ctrl_info,bool enable_events)fs/xfs/libxfs/xfs_btree.h +1
-1@@ -301,7 +301,7 @@ struct xfs_btree_curstatic inline size_t xfs_btree_cur_sizeof (unsigned int nlevels) { -return struct_size ((struct xfs_btree_cur *)NULL , bc_levels, nlevels); + return struct_size_t(struct xfs_btree_cur, bc_levels, nlevels); }/* cursor flags */ fs/xfs/scrub/btree.h +1
-1@@ -60,7 +60,7 @@ struct xchk_btree {static inline size_t xchk_btree_sizeof (unsigned int nlevels) { -return struct_size ((struct xchk_btree *)NULL , lastkey, nlevels -1 ); + return struct_size_t(struct xchk_btree, lastkey, nlevels - 1); } int xchk_btree(struct xfs_scrub *sc, struct xfs_btree_cur *cur,include/linux/overflow.h +17
-1@@ -283,7 +283,7 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* @member: Name of the array member. * @count: Number of elements in the array. * -* Calculates size of memory needed for structure @p followed by an +* Calculates size of memory needed for structure of @p followed by an * array of @count number of @member elements. * * Return: number of bytes needed or SIZE_MAX on overflow. @@ -293,4 +293,20 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) sizeof(*(p)) + flex_array_size(p, member, count), \ size_add(sizeof(*(p)), flex_array_size(p, member, count))) +/** +* struct_size_t() - Calculate size of structure with trailing flexible array +* @type: structure type name. +* @member: Name of the array member. +* @count: Number of elements in the array. +* +* Calculates size of memory needed for structure @type followed by an +* array of @count number of @member elements. Prefer using struct_size() +* when possible instead, to keep calculations associated with a specific +* instance variable of type @type. +* +* Return: number of bytes needed or SIZE_MAX on overflow. +*/ +#define struct_size_t (type, member, count) \ + struct_size((type *)NULL, member, count) +#endif /* __LINUX_OVERFLOW_H */ lib/overflow_kunit.c +1
-1@@ -649,7 +649,7 @@ struct __test_flex_array {static void overflow_size_helpers_test (struct kunit *test) {/* Make sure struct_size() can be used in a constant expression. */ -u8 ce_array[struct_size ((struct __test_flex_array *)0 , data,55 )]; + u8 ce_array[struct_size_t(struct __test_flex_array, data, 55)];struct __test_flex_array *obj;int count =0 ;int var;Signature · 6 lines
2.34.1 _______________________________________________ Intel-wired-lan mailing list [email protected] https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Kees,
Quoted 5 lines
While
struct_size()is normally used in situations where the structure type already has a pointer instance, there are places where no variable is available. In the past, this has been worked around by using a typed NULL first argument, but this is a bit ugly. Add a helper to do this, and replace the handful of instances of the code pattern with it.
SCSI bits look fine to me.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
Looks good:
Reviewed-by Christoph Hellwig <[email protected]>
Quoted 32 lines · On Mon, 22 May 2023 14:18:13 -0700 Kees Cook wrote:
drivers/net/ethernet/intel/ice/ice_ddp.h +3
-3@@ -185,7 +185,7 @@ struct ice_buf_hdr {#define ICE_MAX_ENTRIES_IN_BUF (hd_sz, ent_sz) \ ((ICE_PKG_BUF_SIZE - \ - struct_size((struct ice_buf_hdr *)0, section_entry, 1) - (hd_sz)) / \ + struct_size_t(struct ice_buf_hdr, section_entry, 1) - (hd_sz)) / \ (ent_sz))/* ice package section IDs */ @@ -297,7 +297,7 @@ struct ice_label_section { };#define ICE_MAX_LABELS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_label_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_label_section, \ label, 1) - \ sizeof(struct ice_label), \ sizeof(struct ice_label))@@ -352,7 +352,7 @@ struct ice_boost_tcam_section { };#define ICE_MAX_BST_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_boost_tcam_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_boost_tcam_section, \ tcam, 1) - \ sizeof(struct ice_boost_tcam_entry), \ sizeof(struct ice_boost_tcam_entry))
drivers/net/ethernet/intel/ice/ice_ddp.h struct ice_marker_ptype_tcam_section { · line 378
Quoted 9 lines
drivers/net/ethernet/intel/ice/ice_ddp.h +1
-2@@ -372,8 +372,7 @@ struct ice_marker_ptype_tcam_section { };#define ICE_MAX_MARKER_PTYPE_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF( \ - struct_size((struct ice_marker_ptype_tcam_section *)0, tcam, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_marker_ptype_tcam_section, tcam, \ 1) - \ sizeof(struct ice_marker_ptype_tcam_entry), \ sizeof(struct ice_marker_ptype_tcam_entry))
Acked-by Jakub Kicinski <[email protected]>
but Intel ICE folks please speak up if this has a high chance of conflicts, I think I've seen some ICE DDP patches flying around :(
Alexander Lobakin
Reviewed-byFrom: Jakub Kicinski <[email protected]>
Date: Tue, 23 May 2023 20:53:54 -0700
Quoted 45 lines · On Mon, 22 May 2023 14:18:13 -0700 Kees Cook wrote:
Quoted 32 lines
drivers/net/ethernet/intel/ice/ice_ddp.h +3
-3@@ -185,7 +185,7 @@ struct ice_buf_hdr {#define ICE_MAX_ENTRIES_IN_BUF (hd_sz, ent_sz) \ ((ICE_PKG_BUF_SIZE - \ - struct_size((struct ice_buf_hdr *)0, section_entry, 1) - (hd_sz)) / \ + struct_size_t(struct ice_buf_hdr, section_entry, 1) - (hd_sz)) / \ (ent_sz))/* ice package section IDs */ @@ -297,7 +297,7 @@ struct ice_label_section { };#define ICE_MAX_LABELS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_label_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_label_section, \ label, 1) - \ sizeof(struct ice_label), \ sizeof(struct ice_label))@@ -352,7 +352,7 @@ struct ice_boost_tcam_section { };#define ICE_MAX_BST_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_boost_tcam_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_boost_tcam_section, \ tcam, 1) - \ sizeof(struct ice_boost_tcam_entry), \ sizeof(struct ice_boost_tcam_entry))drivers/net/ethernet/intel/ice/ice_ddp.h struct ice_marker_ptype_tcam_section { · line 378
Quoted 9 lines
drivers/net/ethernet/intel/ice/ice_ddp.h +1
-2@@ -372,8 +372,7 @@ struct ice_marker_ptype_tcam_section { };#define ICE_MAX_MARKER_PTYPE_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF( \ - struct_size((struct ice_marker_ptype_tcam_section *)0, tcam, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_marker_ptype_tcam_section, tcam, \ 1) - \ sizeof(struct ice_marker_ptype_tcam_entry), \ sizeof(struct ice_marker_ptype_tcam_entry))Acked-by Jakub Kicinski <[email protected]>
Quoted 2 lines
but Intel ICE folks please speak up if this has a high chance of conflicts, I think I've seen some ICE DDP patches flying around :(
I haven't found anything that would conflict with this, esp. since it implies no functional changes.
I agree it's been much needed, great, thanks!
Reviewed-by Alexander Lobakin <[email protected]>
Olek
Tony Nguyen
maintainerQuoted 48 lines · Alexander Lobakin
From: Jakub Kicinski <[email protected]>
Date: Tue, 23 May 2023 20:53:54 -0700Quoted 45 lines · On Mon, 22 May 2023 14:18:13 -0700 Kees Cook wrote:
Quoted 32 lines
drivers/net/ethernet/intel/ice/ice_ddp.h +3
-3@@ -185,7 +185,7 @@ struct ice_buf_hdr {#define ICE_MAX_ENTRIES_IN_BUF (hd_sz, ent_sz) \ ((ICE_PKG_BUF_SIZE - \ - struct_size((struct ice_buf_hdr *)0, section_entry, 1) - (hd_sz)) / \ + struct_size_t(struct ice_buf_hdr, section_entry, 1) - (hd_sz)) / \ (ent_sz))/* ice package section IDs */ @@ -297,7 +297,7 @@ struct ice_label_section { };#define ICE_MAX_LABELS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_label_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_label_section, \ label, 1) - \ sizeof(struct ice_label), \ sizeof(struct ice_label))@@ -352,7 +352,7 @@ struct ice_boost_tcam_section { };#define ICE_MAX_BST_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF(struct_size((struct ice_boost_tcam_section *)0, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_boost_tcam_section, \ tcam, 1) - \ sizeof(struct ice_boost_tcam_entry), \ sizeof(struct ice_boost_tcam_entry))drivers/net/ethernet/intel/ice/ice_ddp.h struct ice_marker_ptype_tcam_section { · line 378
Quoted 9 lines
drivers/net/ethernet/intel/ice/ice_ddp.h +1
-2@@ -372,8 +372,7 @@ struct ice_marker_ptype_tcam_section { };#define ICE_MAX_MARKER_PTYPE_TCAMS_IN_BUF \ - ICE_MAX_ENTRIES_IN_BUF( \ - struct_size((struct ice_marker_ptype_tcam_section *)0, tcam, \ + ICE_MAX_ENTRIES_IN_BUF(struct_size_t(struct ice_marker_ptype_tcam_section, tcam, \ 1) - \ sizeof(struct ice_marker_ptype_tcam_entry), \ sizeof(struct ice_marker_ptype_tcam_entry))Acked-by Jakub Kicinski <[email protected]>
Quoted 5 lines
Quoted 2 lines
but Intel ICE folks please speak up if this has a high chance of conflicts, I think I've seen some ICE DDP patches flying around :(
I haven't found anything that would conflict with this, esp. since it implies no functional changes.
Same here. I'm not seeing any conflicts with the patches I'm aware of.
Thanks,
Tony
_______________________________________________
Intel-wired-lan mailing list
[email protected]
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan