linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] treewide: Use sizeof_member() macro
@ 2019-10-10 23:23 Kees Cook
  2019-10-10 23:23 ` [PATCH v2 1/4] MIPS: OCTEON: Replace SIZEOF_FIELD() macro Kees Cook
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Kees Cook @ 2019-10-10 23:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kees Cook, Pankaj Bharadiya, Alexey Dobriyan, Linus Torvalds,
	David S. Miller, Randy Dunlap, Andrew Morton, netdev, linux-arch,
	linux-fsdevel

This is the tree I'll be carrying for the v5.5 merge window to replace
the various struct member sizeof macros with sizeof_member(). The CC list
was insane, so I trimmed it to the major areas that get touched. Dave,
I converted your "no objection"[1] into an Acked-by; please yell if this
is not desired.

Also note that the MIPS patch has been sent separately since it didn't
NEED to depend on this series.

Thanks!

-Kees

[1] https://lore.kernel.org/lkml/20191002.132121.402975401040540710.davem@davemloft.net

v2: rearrange further, make sure Dave Miller is okay with changes
v1: https://lore.kernel.org/lkml/201909261026.6E3381876C@keescook

Pankaj Bharadiya (4):
  MIPS: OCTEON: Replace SIZEOF_FIELD() macro
  linux/stddef.h: Add sizeof_member() macro
  treewide: Use sizeof_member() macro
  include: Remove FIELD_SIZEOF() and sizeof_field() macros

 Documentation/process/coding-style.rst        |   2 +-
 .../it_IT/process/coding-style.rst            |   2 +-
 .../zh_CN/process/coding-style.rst            |   2 +-
 arch/arc/kernel/unwind.c                      |   6 +-
 arch/arm64/include/asm/processor.h            |  10 +-
 .../cavium-octeon/executive/cvmx-bootmem.c    |   9 +-
 arch/powerpc/net/bpf_jit32.h                  |   4 +-
 arch/powerpc/net/bpf_jit_comp.c               |  16 +-
 arch/sparc/net/bpf_jit_comp_32.c              |   8 +-
 arch/x86/kernel/fpu/xstate.c                  |   2 +-
 block/blk-core.c                              |   4 +-
 crypto/adiantum.c                             |   4 +-
 crypto/essiv.c                                |   2 +-
 drivers/firmware/efi/efi.c                    |   2 +-
 drivers/gpu/drm/i915/gvt/scheduler.c          |   2 +-
 drivers/infiniband/hw/efa/efa_verbs.c         |   2 +-
 drivers/infiniband/hw/hfi1/sdma.c             |   2 +-
 drivers/infiniband/hw/hfi1/verbs.h            |   4 +-
 .../ulp/opa_vnic/opa_vnic_ethtool.c           |   2 +-
 drivers/input/keyboard/applespi.c             |   2 +-
 drivers/md/raid5-ppl.c                        |   2 +-
 drivers/media/platform/omap3isp/isppreview.c  |  24 +--
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c  |   4 +-
 .../ethernet/cavium/liquidio/octeon_console.c |  16 +-
 .../net/ethernet/emulex/benet/be_ethtool.c    |   2 +-
 .../hisilicon/hns3/hns3pf/hclge_main.c        |   2 +-
 .../ethernet/hisilicon/hns3/hns3pf/hclge_tm.c |   2 +-
 .../net/ethernet/huawei/hinic/hinic_ethtool.c |   8 +-
 .../net/ethernet/intel/fm10k/fm10k_ethtool.c  |   2 +-
 .../net/ethernet/intel/i40e/i40e_ethtool.c    |   2 +-
 .../net/ethernet/intel/i40e/i40e_lan_hmc.c    |   2 +-
 .../net/ethernet/intel/iavf/iavf_ethtool.c    |   2 +-
 drivers/net/ethernet/intel/ice/ice_ethtool.c  |  10 +-
 .../net/ethernet/intel/ice/ice_lan_tx_rx.h    |   2 +-
 drivers/net/ethernet/intel/igb/igb_ethtool.c  |   4 +-
 drivers/net/ethernet/intel/igc/igc_ethtool.c  |   4 +-
 .../net/ethernet/intel/ixgb/ixgb_ethtool.c    |   4 +-
 drivers/net/ethernet/intel/ixgbevf/ethtool.c  |   4 +-
 drivers/net/ethernet/marvell/mv643xx_eth.c    |   4 +-
 .../net/ethernet/mellanox/mlx4/en_ethtool.c   |   2 +-
 .../ethernet/mellanox/mlx5/core/fpga/ipsec.c  |   6 +-
 .../net/ethernet/mellanox/mlx5/core/fs_core.c |   4 +-
 .../ethernet/mellanox/mlxsw/spectrum_fid.c    |   4 +-
 .../ethernet/mellanox/mlxsw/spectrum_ptp.c    |   2 +-
 drivers/net/ethernet/netronome/nfp/bpf/jit.c  |  10 +-
 drivers/net/ethernet/netronome/nfp/bpf/main.c |   2 +-
 .../net/ethernet/netronome/nfp/bpf/offload.c  |   2 +-
 .../net/ethernet/netronome/nfp/flower/main.h  |   2 +-
 .../oki-semi/pch_gbe/pch_gbe_ethtool.c        |   2 +-
 drivers/net/ethernet/qlogic/qede/qede.h       |   2 +-
 .../ethernet/qlogic/qlcnic/qlcnic_ethtool.c   |   2 +-
 .../ethernet/samsung/sxgbe/sxgbe_ethtool.c    |   2 +-
 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |   4 +-
 drivers/net/ethernet/ti/cpsw_ethtool.c        |   6 +-
 drivers/net/ethernet/ti/netcp_ethss.c         |  32 ++--
 drivers/net/fjes/fjes_ethtool.c               |   2 +-
 drivers/net/geneve.c                          |   2 +-
 drivers/net/hyperv/netvsc_drv.c               |   2 +-
 drivers/net/usb/sierra_net.c                  |   2 +-
 drivers/net/usb/usbnet.c                      |   2 +-
 drivers/net/vxlan.c                           |   4 +-
 .../net/wireless/marvell/libertas/debugfs.c   |   2 +-
 drivers/net/wireless/marvell/mwifiex/util.h   |   4 +-
 drivers/s390/net/qeth_core_main.c             |   2 +-
 drivers/s390/net/qeth_core_mpc.h              |  10 +-
 drivers/scsi/aacraid/aachba.c                 |   4 +-
 drivers/scsi/be2iscsi/be_cmds.h               |   2 +-
 drivers/scsi/cxgbi/libcxgbi.c                 |   2 +-
 drivers/scsi/smartpqi/smartpqi_init.c         |   6 +-
 drivers/staging/qlge/qlge_ethtool.c           |   2 +-
 drivers/target/iscsi/cxgbit/cxgbit_main.c     |   2 +-
 drivers/usb/atm/usbatm.c                      |   2 +-
 drivers/usb/gadget/function/f_fs.c            |   2 +-
 fs/befs/linuxvfs.c                            |   2 +-
 fs/crypto/keyring.c                           |   2 +-
 fs/ext2/super.c                               |   2 +-
 fs/ext4/super.c                               |   2 +-
 fs/freevxfs/vxfs_super.c                      |   2 +-
 fs/fuse/virtio_fs.c                           |   2 +-
 fs/orangefs/super.c                           |   2 +-
 fs/ufs/super.c                                |   2 +-
 fs/verity/enable.c                            |   2 +-
 include/linux/filter.h                        |  12 +-
 include/linux/kernel.h                        |   9 --
 include/linux/kvm_host.h                      |   2 +-
 include/linux/phy_led_triggers.h              |   2 +-
 include/linux/slab.h                          |   2 +-
 include/linux/stddef.h                        |  13 +-
 include/net/garp.h                            |   2 +-
 include/net/ip_tunnels.h                      |   6 +-
 include/net/mrp.h                             |   2 +-
 include/net/netfilter/nf_conntrack_helper.h   |   2 +-
 include/net/netfilter/nf_tables_core.h        |   2 +-
 include/net/sock.h                            |   2 +-
 ipc/util.c                                    |   2 +-
 kernel/bpf/cgroup.c                           |   2 +-
 kernel/bpf/local_storage.c                    |   4 +-
 kernel/fork.c                                 |   2 +-
 kernel/signal.c                               |  12 +-
 kernel/utsname.c                              |   2 +-
 net/802/mrp.c                                 |   6 +-
 net/batman-adv/main.c                         |   2 +-
 net/bpf/test_run.c                            |   6 +-
 net/bridge/br.c                               |   2 +-
 net/caif/caif_socket.c                        |   2 +-
 net/core/dev.c                                |   2 +-
 net/core/filter.c                             | 140 +++++++++---------
 net/core/flow_dissector.c                     |  10 +-
 net/core/skbuff.c                             |   2 +-
 net/core/xdp.c                                |   4 +-
 net/dccp/proto.c                              |   2 +-
 net/ipv4/ip_gre.c                             |   4 +-
 net/ipv4/ip_vti.c                             |   4 +-
 net/ipv4/raw.c                                |   2 +-
 net/ipv4/tcp.c                                |   2 +-
 net/ipv6/ip6_gre.c                            |   4 +-
 net/ipv6/raw.c                                |   2 +-
 net/iucv/af_iucv.c                            |   2 +-
 net/netfilter/nf_tables_api.c                 |   4 +-
 net/netfilter/nfnetlink_cthelper.c            |   2 +-
 net/netfilter/nft_ct.c                        |  12 +-
 net/netfilter/nft_masq.c                      |   2 +-
 net/netfilter/nft_nat.c                       |   6 +-
 net/netfilter/nft_redir.c                     |   2 +-
 net/netfilter/nft_tproxy.c                    |   4 +-
 net/netfilter/xt_RATEEST.c                    |   2 +-
 net/netlink/af_netlink.c                      |   2 +-
 net/openvswitch/datapath.c                    |   2 +-
 net/openvswitch/flow.h                        |   4 +-
 net/rxrpc/af_rxrpc.c                          |   2 +-
 net/sched/act_ct.c                            |   4 +-
 net/sched/cls_flower.c                        |   2 +-
 net/sctp/socket.c                             |   4 +-
 net/unix/af_unix.c                            |   2 +-
 security/integrity/ima/ima_policy.c           |   4 +-
 sound/soc/codecs/hdmi-codec.c                 |   2 +-
 tools/testing/selftests/bpf/bpf_util.h        |   6 +-
 virt/kvm/kvm_main.c                           |   2 +-
 138 files changed, 339 insertions(+), 352 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/4] MIPS: OCTEON: Replace SIZEOF_FIELD() macro
  2019-10-10 23:23 [PATCH v2 0/4] treewide: Use sizeof_member() macro Kees Cook
@ 2019-10-10 23:23 ` Kees Cook
  2019-10-10 23:23 ` [PATCH v2 2/4] linux/stddef.h: Add sizeof_member() macro Kees Cook
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2019-10-10 23:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kees Cook, Pankaj Bharadiya, Alexey Dobriyan, Linus Torvalds,
	David S. Miller, Randy Dunlap, Andrew Morton, netdev, linux-arch,
	linux-fsdevel

From: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>

In preparation for switching to a standard sizeof_member() macro to find the
size of a member of a struct, remove the custom SIZEOF_FIELD() macro and use
the more common FIELD_SIZEOF() instead. Later patches will globally replace
FIELD_SIZEOF() and sizeof_field() with the more accurate sizeof_member().

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Link: https://lore.kernel.org/r/20190924105839.110713-4-pankaj.laxminarayan.bharadiya@intel.com
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/mips/cavium-octeon/executive/cvmx-bootmem.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/mips/cavium-octeon/executive/cvmx-bootmem.c b/arch/mips/cavium-octeon/executive/cvmx-bootmem.c
index ba8f82a29a81..44b506a14666 100644
--- a/arch/mips/cavium-octeon/executive/cvmx-bootmem.c
+++ b/arch/mips/cavium-octeon/executive/cvmx-bootmem.c
@@ -44,13 +44,6 @@ static struct cvmx_bootmem_desc *cvmx_bootmem_desc;
 
 /* See header file for descriptions of functions */
 
-/**
- * This macro returns the size of a member of a structure.
- * Logically it is the same as "sizeof(s::field)" in C++, but
- * C lacks the "::" operator.
- */
-#define SIZEOF_FIELD(s, field) sizeof(((s *)NULL)->field)
-
 /**
  * This macro returns a member of the
  * cvmx_bootmem_named_block_desc_t structure. These members can't
@@ -65,7 +58,7 @@ static struct cvmx_bootmem_desc *cvmx_bootmem_desc;
 #define CVMX_BOOTMEM_NAMED_GET_FIELD(addr, field)			\
 	__cvmx_bootmem_desc_get(addr,					\
 		offsetof(struct cvmx_bootmem_named_block_desc, field),	\
-		SIZEOF_FIELD(struct cvmx_bootmem_named_block_desc, field))
+		FIELD_SIZEOF(struct cvmx_bootmem_named_block_desc, field))
 
 /**
  * This function is the implementation of the get macros defined
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/4] linux/stddef.h: Add sizeof_member() macro
  2019-10-10 23:23 [PATCH v2 0/4] treewide: Use sizeof_member() macro Kees Cook
  2019-10-10 23:23 ` [PATCH v2 1/4] MIPS: OCTEON: Replace SIZEOF_FIELD() macro Kees Cook
@ 2019-10-10 23:23 ` Kees Cook
  2019-10-10 23:23 ` [PATCH v2 4/4] include: Remove FIELD_SIZEOF() and sizeof_field() macros Kees Cook
       [not found] ` <20191010232345.26594-4-keescook@chromium.org>
  3 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2019-10-10 23:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kees Cook, Pankaj Bharadiya, Alexey Dobriyan, Linus Torvalds,
	David S. Miller, Randy Dunlap, Andrew Morton, netdev, linux-arch,
	linux-fsdevel

From: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>

At present we have 2 different macros to calculate the size of a member
of a struct: FIELD_SIZEOF() and sizeof_field(). As a prerequisite to
bringing uniformity to the entire kernel source tree, add sizeof_member()
macro as it is both more pleasant (not upper case) and more correct
(sizeof()-family cannot operate on bit fields; this is meant to operate
on struct members), as discussed[1].

Future patches will replace all occurrences of above macros with
sizeof_member().

[1] https://www.openwall.com/lists/kernel-hardening/2019/07/02/2

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Link: https://lore.kernel.org/r/20190924105839.110713-2-pankaj.laxminarayan.bharadiya@intel.com
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/stddef.h                 | 13 ++++++++++++-
 tools/testing/selftests/bpf/bpf_util.h |  6 +++---
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 998a4ba28eba..ecadb736c853 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -27,6 +27,17 @@ enum {
  */
 #define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
 
+/**
+ * sizeof_member(TYPE, MEMBER) - get the size of a struct's member
+ *
+ * @TYPE: the target struct
+ * @MEMBER: the target struct's member
+ *
+ * Return: the size of @MEMBER in the struct definition without having a
+ * declared instance of @TYPE.
+ */
+#define sizeof_member(TYPE, MEMBER)	(sizeof(((TYPE *)0)->MEMBER))
+
 /**
  * offsetofend(TYPE, MEMBER)
  *
@@ -34,6 +45,6 @@ enum {
  * @MEMBER: The member within the structure to get the end offset of
  */
 #define offsetofend(TYPE, MEMBER) \
-	(offsetof(TYPE, MEMBER)	+ sizeof_field(TYPE, MEMBER))
+	(offsetof(TYPE, MEMBER)	+ sizeof_member(TYPE, MEMBER))
 
 #endif
diff --git a/tools/testing/selftests/bpf/bpf_util.h b/tools/testing/selftests/bpf/bpf_util.h
index ec219f84e041..6b4b3e24ba9f 100644
--- a/tools/testing/selftests/bpf/bpf_util.h
+++ b/tools/testing/selftests/bpf/bpf_util.h
@@ -31,13 +31,13 @@ static inline unsigned int bpf_num_possible_cpus(void)
 # define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 #endif
 
-#ifndef sizeof_field
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
+#ifndef sizeof_member
+#define sizeof_member(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
 #endif
 
 #ifndef offsetofend
 #define offsetofend(TYPE, MEMBER) \
-	(offsetof(TYPE, MEMBER)	+ sizeof_field(TYPE, MEMBER))
+	(offsetof(TYPE, MEMBER)	+ sizeof_member(TYPE, MEMBER))
 #endif
 
 #endif /* __BPF_UTIL__ */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 4/4] include: Remove FIELD_SIZEOF() and sizeof_field() macros
  2019-10-10 23:23 [PATCH v2 0/4] treewide: Use sizeof_member() macro Kees Cook
  2019-10-10 23:23 ` [PATCH v2 1/4] MIPS: OCTEON: Replace SIZEOF_FIELD() macro Kees Cook
  2019-10-10 23:23 ` [PATCH v2 2/4] linux/stddef.h: Add sizeof_member() macro Kees Cook
@ 2019-10-10 23:23 ` Kees Cook
       [not found] ` <20191010232345.26594-4-keescook@chromium.org>
  3 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2019-10-10 23:23 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kees Cook, Pankaj Bharadiya, Alexey Dobriyan, Linus Torvalds,
	David S. Miller, Randy Dunlap, Andrew Morton, netdev, linux-arch,
	linux-fsdevel

From: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>

The common function used to find struct member size is sizeof_member().
Remove the now unused FIELD_SIZEOF() and sizeof_field() macros.

Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
Link: https://lore.kernel.org/r/20190924105839.110713-5-pankaj.laxminarayan.bharadiya@intel.com
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/kernel.h | 9 ---------
 include/linux/stddef.h | 8 --------
 2 files changed, 17 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d83d403dac2e..d67020250d75 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -79,15 +79,6 @@
  */
 #define round_down(x, y) ((x) & ~__round_mask(x, y))
 
-/**
- * FIELD_SIZEOF - get the size of a struct's field
- * @t: the target struct
- * @f: the target struct's field
- * Return: the size of @f in the struct definition without having a
- * declared instance of @t.
- */
-#define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
-
 #define typeof_member(T, m)	typeof(((T*)0)->m)
 
 #define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index ecadb736c853..74b5e644d50a 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -19,14 +19,6 @@ enum {
 #define offsetof(TYPE, MEMBER)	((size_t)&((TYPE *)0)->MEMBER)
 #endif
 
-/**
- * sizeof_field(TYPE, MEMBER)
- *
- * @TYPE: The structure containing the field of interest
- * @MEMBER: The field to return the size of
- */
-#define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
-
 /**
  * sizeof_member(TYPE, MEMBER) - get the size of a struct's member
  *
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/4] treewide: Use sizeof_member() macro
       [not found] ` <20191010232345.26594-4-keescook@chromium.org>
@ 2019-10-10 23:50   ` Joe Perches
  2019-10-29 22:30     ` Kees Cook
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2019-10-10 23:50 UTC (permalink / raw)
  To: Kees Cook, linux-kernel
  Cc: Pankaj Bharadiya, Alexey Dobriyan, Linus Torvalds,
	David S. Miller, Randy Dunlap, Andrew Morton, netdev, linux-arch,
	linux-fsdevel

On Thu, 2019-10-10 at 16:23 -0700, Kees Cook wrote:
> From: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> 
> Replace all the occurrences of FIELD_SIZEOF() and sizeof_field() with
> sizeof_member() except at places where these are defined. Later patches
> will remove the unused definitions.
> 
> This patch is generated using following script:
> 
> EXCLUDE_FILES="include/linux/stddef.h|include/linux/kernel.h"
> 
> git grep -l -e "\bFIELD_SIZEOF\b" -e "\bsizeof_field\b" | while read file;
> do
> 
> 	if [[ "$file" =~ $EXCLUDE_FILES ]]; then
> 		continue
> 	fi
> 	sed -i  -e 's/\bFIELD_SIZEOF\b/sizeof_member/g' \
> 		-e 's/\bsizeof_field\b/sizeof_member/g' \
> 		$file;
> done

While the sed works, a cocci script would perhaps
be better as multi line argument realignment would
also occur.

$ cat sizeof_member.cocci
@@
@@

-	FIELD_SIZEOF
+	sizeof_member

@@
@@

-	sizeof_field
+	sizeof_member
$

For instance, this sed produces:

diff --git a/crypto/adiantum.c b/crypto/adiantum.c
@@ -435,10 +435,10 @@ static int adiantum_init_tfm(struct crypto_skcipher *tfm)
 
 	BUILD_BUG_ON(offsetofend(struct adiantum_request_ctx, u) !=
 		     sizeof(struct adiantum_request_ctx));
-	subreq_size = max(FIELD_SIZEOF(struct adiantum_request_ctx,
+	subreq_size = max(sizeof_member(struct adiantum_request_ctx,
 				       u.hash_desc) +
 			  crypto_shash_descsize(hash),
-			  FIELD_SIZEOF(struct adiantum_request_ctx,
+			  sizeof_member(struct adiantum_request_ctx,
 				       u.streamcipher_req) +
 			  crypto_skcipher_reqsize(streamcipher));
 

where the cocci script produces:

--- crypto/adiantum.c
+++ /tmp/cocci-output-22881-d8186c-adiantum.c
@@ -435,11 +435,11 @@ static int adiantum_init_tfm(struct cryp
 
 	BUILD_BUG_ON(offsetofend(struct adiantum_request_ctx, u) !=
 		     sizeof(struct adiantum_request_ctx));
-	subreq_size = max(FIELD_SIZEOF(struct adiantum_request_ctx,
-				       u.hash_desc) +
+	subreq_size = max(sizeof_member(struct adiantum_request_ctx,
+					u.hash_desc) +
 			  crypto_shash_descsize(hash),
-			  FIELD_SIZEOF(struct adiantum_request_ctx,
-				       u.streamcipher_req) +
+			  sizeof_member(struct adiantum_request_ctx,
+					u.streamcipher_req) +
 			  crypto_skcipher_reqsize(streamcipher));
 
 	crypto_skcipher_set_reqsize(tfm,



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/4] treewide: Use sizeof_member() macro
  2019-10-10 23:50   ` [PATCH v2 3/4] treewide: Use sizeof_member() macro Joe Perches
@ 2019-10-29 22:30     ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2019-10-29 22:30 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, Pankaj Bharadiya, Alexey Dobriyan, Linus Torvalds,
	David S. Miller, Randy Dunlap, Andrew Morton, netdev, linux-arch,
	linux-fsdevel

On Thu, Oct 10, 2019 at 04:50:27PM -0700, Joe Perches wrote:
> On Thu, 2019-10-10 at 16:23 -0700, Kees Cook wrote:
> > From: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> > 
> > Replace all the occurrences of FIELD_SIZEOF() and sizeof_field() with
> > sizeof_member() except at places where these are defined. Later patches
> > will remove the unused definitions.
> > 
> > This patch is generated using following script:
> > 
> > EXCLUDE_FILES="include/linux/stddef.h|include/linux/kernel.h"
> > 
> > git grep -l -e "\bFIELD_SIZEOF\b" -e "\bsizeof_field\b" | while read file;
> > do
> > 
> > 	if [[ "$file" =~ $EXCLUDE_FILES ]]; then
> > 		continue
> > 	fi
> > 	sed -i  -e 's/\bFIELD_SIZEOF\b/sizeof_member/g' \
> > 		-e 's/\bsizeof_field\b/sizeof_member/g' \
> > 		$file;
> > done
> 
> While the sed works, a cocci script would perhaps
> be better as multi line argument realignment would
> also occur.
> 
> $ cat sizeof_member.cocci
> @@
> @@
> 
> -	FIELD_SIZEOF
> +	sizeof_member
> 
> @@
> @@
> 
> -	sizeof_field
> +	sizeof_member
> $
> 
> For instance, this sed produces:
> 
> diff --git a/crypto/adiantum.c b/crypto/adiantum.c
> @@ -435,10 +435,10 @@ static int adiantum_init_tfm(struct crypto_skcipher *tfm)
>  
>  	BUILD_BUG_ON(offsetofend(struct adiantum_request_ctx, u) !=
>  		     sizeof(struct adiantum_request_ctx));
> -	subreq_size = max(FIELD_SIZEOF(struct adiantum_request_ctx,
> +	subreq_size = max(sizeof_member(struct adiantum_request_ctx,
>  				       u.hash_desc) +
>  			  crypto_shash_descsize(hash),
> -			  FIELD_SIZEOF(struct adiantum_request_ctx,
> +			  sizeof_member(struct adiantum_request_ctx,
>  				       u.streamcipher_req) +
>  			  crypto_skcipher_reqsize(streamcipher));
>  
> 
> where the cocci script produces:
> 
> --- crypto/adiantum.c
> +++ /tmp/cocci-output-22881-d8186c-adiantum.c
> @@ -435,11 +435,11 @@ static int adiantum_init_tfm(struct cryp
>  
>  	BUILD_BUG_ON(offsetofend(struct adiantum_request_ctx, u) !=
>  		     sizeof(struct adiantum_request_ctx));
> -	subreq_size = max(FIELD_SIZEOF(struct adiantum_request_ctx,
> -				       u.hash_desc) +
> +	subreq_size = max(sizeof_member(struct adiantum_request_ctx,
> +					u.hash_desc) +
>  			  crypto_shash_descsize(hash),
> -			  FIELD_SIZEOF(struct adiantum_request_ctx,
> -				       u.streamcipher_req) +
> +			  sizeof_member(struct adiantum_request_ctx,
> +					u.streamcipher_req) +
>  			  crypto_skcipher_reqsize(streamcipher));
>  
>  	crypto_skcipher_set_reqsize(tfm,

I played with this a bit, and it seems Coccinelle can get this very very
wrong:

diff -u -p a/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c
@@ -87,13 +87,13 @@ static const struct rhashtable_params rh
 	 * value is not constant during the lifetime
 	 * of the key object.
 	 */
-	.key_len = FIELD_SIZEOF(struct mlx5_fpga_ipsec_sa_ctx, hw_sa) -
-		   FIELD_SIZEOF(struct mlx5_ifc_fpga_ipsec_sa_v1, cmd),
+	.key_len = sizeof_member(struct mlx5_fpga_ipsec_sa_ctx, hw_sa) -
+	sizeof_member(struct mlx5_ifc_fpga_ipsec_sa_v1, cmd),
 	.key_offset = offsetof(struct mlx5_fpga_ipsec_sa_ctx, hw_sa) +
-		      FIELD_SIZEOF(struct mlx5_ifc_fpga_ipsec_sa_v1, cmd),
-	.head_offset = offsetof(struct mlx5_fpga_ipsec_sa_ctx, hash),
-	.automatic_shrinking = true,
-	.min_size = 1,
+		      sizeof_member(struct mlx5_ifc_fpga_ipsec_sa_v1, cmd),
+		      .head_offset = offsetof(struct mlx5_fpga_ipsec_sa_ctx, hash),
+		      .automatic_shrinking = true,
+		      .min_size = 1,
 };
 
 struct mlx5_fpga_ipsec {


So, since the sed is faster and causes fewer problems, I'll keep it
as-is.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-29 22:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-10 23:23 [PATCH v2 0/4] treewide: Use sizeof_member() macro Kees Cook
2019-10-10 23:23 ` [PATCH v2 1/4] MIPS: OCTEON: Replace SIZEOF_FIELD() macro Kees Cook
2019-10-10 23:23 ` [PATCH v2 2/4] linux/stddef.h: Add sizeof_member() macro Kees Cook
2019-10-10 23:23 ` [PATCH v2 4/4] include: Remove FIELD_SIZEOF() and sizeof_field() macros Kees Cook
     [not found] ` <20191010232345.26594-4-keescook@chromium.org>
2019-10-10 23:50   ` [PATCH v2 3/4] treewide: Use sizeof_member() macro Joe Perches
2019-10-29 22:30     ` Kees Cook

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).