Thread (152 messages) 152 messages, 21 authors, 2021-08-13

Re: [PATCH 48/64] drbd: Use struct_group() to zero algs

From: Bart Van Assche <bvanassche@acm.org>
Date: 2021-07-28 21:46:02
Also in: dri-devel, linux-block, linux-hardening, linux-staging, linux-wireless, lkml, netdev

On 7/27/21 1:58 PM, Kees Cook wrote:
quoted hunk ↗ jump to hunk
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.

Add a struct_group() for the algs so that memset() can correctly reason
about the size.

Signed-off-by: Kees Cook <redacted>
---
  drivers/block/drbd/drbd_main.c     | 3 ++-
  drivers/block/drbd/drbd_protocol.h | 6 ++++--
  drivers/block/drbd/drbd_receiver.c | 3 ++-
  3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 55234a558e98..b824679cfcb2 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -729,7 +729,8 @@ int drbd_send_sync_param(struct drbd_peer_device *peer_device)
  	cmd = apv >= 89 ? P_SYNC_PARAM89 : P_SYNC_PARAM;
  
  	/* initialize verify_alg and csums_alg */
-	memset(p->verify_alg, 0, 2 * SHARED_SECRET_MAX);
+	BUILD_BUG_ON(sizeof(p->algs) != 2 * SHARED_SECRET_MAX);
+	memset(&p->algs, 0, sizeof(p->algs));
  
  	if (get_ldev(peer_device->device)) {
  		dc = rcu_dereference(peer_device->device->ldev->disk_conf);
diff --git a/drivers/block/drbd/drbd_protocol.h b/drivers/block/drbd/drbd_protocol.h
index dea59c92ecc1..a882b65ab5d2 100644
--- a/drivers/block/drbd/drbd_protocol.h
+++ b/drivers/block/drbd/drbd_protocol.h
@@ -283,8 +283,10 @@ struct p_rs_param_89 {
  
  struct p_rs_param_95 {
  	u32 resync_rate;
-	char verify_alg[SHARED_SECRET_MAX];
-	char csums_alg[SHARED_SECRET_MAX];
+	struct_group(algs,
+		char verify_alg[SHARED_SECRET_MAX];
+		char csums_alg[SHARED_SECRET_MAX];
+	);
  	u32 c_plan_ahead;
  	u32 c_delay_target;
  	u32 c_fill_target;
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 1f740e42e457..6df2539e215b 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -3921,7 +3921,8 @@ static int receive_SyncParam(struct drbd_connection *connection, struct packet_i
  
  	/* initialize verify_alg and csums_alg */
  	p = pi->data;
-	memset(p->verify_alg, 0, 2 * SHARED_SECRET_MAX);
+	BUILD_BUG_ON(sizeof(p->algs) != 2 * SHARED_SECRET_MAX);
+	memset(&p->algs, 0, sizeof(p->algs));
Using struct_group() introduces complexity. Has it been considered not 
to modify struct p_rs_param_95 and instead to use two memset() calls 
instead of one (one memset() call per member)?

Thanks,

Bart.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help