On 2023-07-05 11:06:46 Wed, Jordan Niethe wrote:

On 26/6/23 5:04 pm, Mahesh Salgaonkar wrote:

quoted

opal_prd_msg_notifier extracts the opal prd message size from the message
header and uses it for allocating opal_prd_msg_queue_item that includes
the correct message size to be copied. However, while running under
CONFIG_FORTIFY_SOURCE=y, it triggers following run-time warning:

[ 6458.234352] memcpy: detected field-spanning write (size 32) of single field "&item->msg" at arch/powerpc/platforms/powernv/opal-prd.c:355 (size 4)
[ 6458.234390] WARNING: CPU: 9 PID: 660 at arch/powerpc/platforms/powernv/opal-prd.c:355 opal_prd_msg_notifier+0x174/0x188 [opal_prd]
[...]
[ 6458.234709] NIP [c00800000e0c0e6c] opal_prd_msg_notifier+0x174/0x188 [opal_prd]
[ 6458.234723] LR [c00800000e0c0e68] opal_prd_msg_notifier+0x170/0x188 [opal_prd]
[ 6458.234736] Call Trace:
[ 6458.234742] [c0000002acb23c10] [c00800000e0c0e68] opal_prd_msg_notifier+0x170/0x188 [opal_prd] (unreliable)
[ 6458.234759] [c0000002acb23ca0] [c00000000019ccc0] notifier_call_chain+0xc0/0x1b0
[ 6458.234774] [c0000002acb23d00] [c00000000019ceac] atomic_notifier_call_chain+0x2c/0x40
[ 6458.234788] [c0000002acb23d20] [c0000000000d69b4] opal_message_notify+0xf4/0x2c0
[...]

Add a flexible array member to avoid false positive run-time warning.

Reported-by: Aneesh Kumar K.V <redacted>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
---
  arch/powerpc/platforms/powernv/opal-prd.c |    7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-prd.c b/arch/powerpc/platforms/powernv/opal-prd.c
index 113bdb151f687..9e2c4775f75f5 100644
--- a/arch/powerpc/platforms/powernv/opal-prd.c
+++ b/arch/powerpc/platforms/powernv/opal-prd.c

@@ -30,7 +30,10 @@
   */
  struct opal_prd_msg_queue_item {
  	struct list_head		list;
-	struct opal_prd_msg_header	msg;
+	union {
+		struct opal_prd_msg_header	msg;
+		DECLARE_FLEX_ARRAY(__u8, msg_flex);
+	};
  };
  static struct device_node *prd_node;

@@ -352,7 +355,7 @@ static int opal_prd_msg_notifier(struct notifier_block *nb,
  	if (!item)
  		return -ENOMEM;
-	memcpy(&item->msg, msg->params, msg_size);
+	memcpy(&item->msg_flex, msg->params, msg_size);

This does silence the warning but it seems like we might be able to go a
little further.

What about not adding that flex array to struct opal_prd_msg_queue_item, but
adding one to struct opal_prd_msg. That is what the data format actually is.

So we'd have something like:


    struct opal_prd_msg  {

        struct opal_prd_msg_header hdr;

        char msg[];

    }


and change things to use that instead?

But that might be more trouble than it is worth,

Yup, it would cause more changes to the code elsewhere as well.

alternatively we can just
do:

	item->msg = *hdr;
	memcpy((char *)item->msg + sizeof(*hdr), (char *)hdr + sizeof(*hdr),
msg_size - sizeof(*hdr));

Agree, this helps, I tried with small change to above code which
also works fine.

	item->msg = *hdr;
	hdr++;
	memcpy((char *)&item->msg + sizeof(*hdr), hdr, msg_size - sizeof(*hdr));

Will send out v2.

Thanks,
-Mahesh.