lkml.org 
[lkml]   [2023]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Intel-wired-lan] [PATCH] overflow: Add struct_size_t() helper
    On Mon, May 22, 2023 at 02:18:13PM -0700, Kees Cook wrote:
    > 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)

    This indeed was much needed.

    Plus, we save 3 characters on each line. :D

    >
    > Suggested-by: Christoph Hellwig <hch@infradead.org>
    > Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
    > Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
    > Cc: "David S. Miller" <davem@davemloft.net>
    > Cc: Eric Dumazet <edumazet@google.com>
    > Cc: Jakub Kicinski <kuba@kernel.org>
    > Cc: Paolo Abeni <pabeni@redhat.com>
    > Cc: James Smart <james.smart@broadcom.com>
    > Cc: Keith Busch <kbusch@kernel.org>
    > Cc: Jens Axboe <axboe@kernel.dk>
    > Cc: Sagi Grimberg <sagi@grimberg.me>
    > Cc: HighPoint Linux Team <linux@highpoint-tech.com>
    > Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
    > Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
    > Cc: Kashyap Desai <kashyap.desai@broadcom.com>
    > Cc: Sumit Saxena <sumit.saxena@broadcom.com>
    > Cc: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
    > Cc: Don Brace <don.brace@microchip.com>
    > Cc: "Darrick J. Wong" <djwong@kernel.org>
    > Cc: Dave Chinner <dchinner@redhat.com>
    > Cc: Guo Xuenan <guoxuenan@huawei.com>
    > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
    > Cc: Nick Desaulniers <ndesaulniers@google.com>
    > Cc: Daniel Latypov <dlatypov@google.com>
    > Cc: kernel test robot <lkp@intel.com>
    > Cc: intel-wired-lan@lists.osuosl.org
    > Cc: netdev@vger.kernel.org
    > Cc: linux-nvme@lists.infradead.org
    > Cc: linux-scsi@vger.kernel.org
    > Cc: megaraidlinux.pdl@broadcom.com
    > Cc: storagedev@microchip.com
    > Cc: linux-xfs@vger.kernel.org
    > Cc: linux-hardening@vger.kernel.org
    > Signed-off-by: Kees Cook <keescook@chromium.org>

    Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

    Thanks!
    --
    Gustavo

    > ---
    > 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(-)
    >
    > diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.h b/drivers/net/ethernet/intel/ice/ice_ddp.h
    > index 37eadb3d27a8..41acfe26df1c 100644
    > --- a/drivers/net/ethernet/intel/ice/ice_ddp.h
    > +++ b/drivers/net/ethernet/intel/ice/ice_ddp.h
    > @@ -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))
    > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
    > index 2ed75923507d..691f2df574ce 100644
    > --- a/drivers/nvme/host/fc.c
    > +++ b/drivers/nvme/host/fc.c
    > @@ -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;
    >
    > diff --git a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
    > index 06ccb51bf6a9..f5334ccbf2ca 100644
    > --- a/drivers/scsi/hptiop.c
    > +++ b/drivers/scsi/hptiop.c
    > @@ -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;
    >
    > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
    > index 317c944c68e3..050eed8e2684 100644
    > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
    > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
    > @@ -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,
    > diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c b/drivers/scsi/megaraid/megaraid_sas_fp.c
    > index 4463a538102a..b8b388a4e28f 100644
    > --- a/drivers/scsi/megaraid/megaraid_sas_fp.c
    > +++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
    > @@ -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",
    > diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
    > index 03de97cd72c2..f4e0aa262164 100644
    > --- a/drivers/scsi/smartpqi/smartpqi_init.c
    > +++ b/drivers/scsi/smartpqi/smartpqi_init.c
    > @@ -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)
    > diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
    > index a2aa36b23e25..4d68a58be160 100644
    > --- a/fs/xfs/libxfs/xfs_btree.h
    > +++ b/fs/xfs/libxfs/xfs_btree.h
    > @@ -301,7 +301,7 @@ struct xfs_btree_cur
    > static 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 */
    > diff --git a/fs/xfs/scrub/btree.h b/fs/xfs/scrub/btree.h
    > index 9d7b9ee8bef4..c32b5fad6174 100644
    > --- a/fs/xfs/scrub/btree.h
    > +++ b/fs/xfs/scrub/btree.h
    > @@ -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,
    > diff --git a/include/linux/overflow.h b/include/linux/overflow.h
    > index 0e33b5cbdb9f..f9b60313eaea 100644
    > --- a/include/linux/overflow.h
    > +++ b/include/linux/overflow.h
    > @@ -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 */
    > diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c
    > index dcd3ba102db6..34db0b3aa502 100644
    > --- a/lib/overflow_kunit.c
    > +++ b/lib/overflow_kunit.c
    > @@ -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;
    > --
    > 2.34.1
    >
    > _______________________________________________
    > Intel-wired-lan mailing list
    > Intel-wired-lan@osuosl.org
    > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

    \
     
     \ /
      Last update: 2023-05-22 23:35    [W:2.723 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site