[PATCH] nfc: pn533: fix fortify warning

[PATCH] nfc: pn533: fix fortify warning

[Dmitry Antipov]

When compiling with gcc version 14.0.0 20231129 (experimental) and
CONFIG_FORTIFY_SOURCE=y, I've noticed the following:

In file included from ./include/linux/string.h:295,
                 from ./include/linux/bitmap.h:12,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/x86/include/asm/paravirt.h:17,
                 from ./arch/x86/include/asm/irqflags.h:60,
                 from ./include/linux/irqflags.h:17,
                 from ./include/linux/rcupdate.h:26,
                 from ./include/linux/rculist.h:11,
                 from ./include/linux/pid.h:5,
                 from ./include/linux/sched.h:14,
                 from ./include/linux/ratelimit.h:6,
                 from ./include/linux/dev_printk.h:16,
                 from ./include/linux/device.h:15,
                 from drivers/nfc/pn533/pn533.c:9:
In function 'fortify_memcpy_chk',
    inlined from 'pn533_target_found_felica' at drivers/nfc/pn533/pn533.c:781:2:
./include/linux/fortify-string.h:588:25: warning: call to '__read_overflow2_field'
declared with attribute warning: detected read beyond size of field (2nd parameter);
maybe use struct_group()? [-Wattribute-warning]
  588 |                         __read_overflow2_field(q_size_field, size);

Here the fortification logic interprets call to 'memcpy()' as an attempt
to copy an amount of data which exceeds the size of the specified field
(9 bytes from 1-byte 'opcode') and thus issues an overread warning -
which is silenced by using the convenient 'struct_group()' quirk.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/nfc/pn533/pn533.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index b19c39dcfbd9..7fb0f6c004f7 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -740,8 +740,10 @@ static int pn533_target_found_type_a(struct nfc_target *nfc_tgt, u8 *tgt_data,
 
 struct pn533_target_felica {
 	u8 pol_res;
-	u8 opcode;
-	u8 nfcid2[NFC_NFCID2_MAXSIZE];
+	struct_group(sensf,
+		u8 opcode;
+		u8 nfcid2[NFC_NFCID2_MAXSIZE];
+	);
 	u8 pad[8];
 	/* optional */
 	u8 syst_code[];
@@ -778,8 +780,9 @@ static int pn533_target_found_felica(struct nfc_target *nfc_tgt, u8 *tgt_data,
 	else
 		nfc_tgt->supported_protocols = NFC_PROTO_FELICA_MASK;
 
-	memcpy(nfc_tgt->sensf_res, &tgt_felica->opcode, 9);
-	nfc_tgt->sensf_res_len = 9;
+	memcpy(nfc_tgt->sensf_res, &tgt_felica->sensf,
+	       sizeof(tgt_felica->sensf));
+	nfc_tgt->sensf_res_len = sizeof(tgt_felica->sensf);
 
 	memcpy(nfc_tgt->nfcid2, tgt_felica->nfcid2, NFC_NFCID2_MAXSIZE);
 	nfc_tgt->nfcid2_len = NFC_NFCID2_MAXSIZE;
-- 
2.43.0

Re: [PATCH] nfc: pn533: fix fortify warning

[Krzysztof Kozlowski]

On 29/11/2023 18:03, Dmitry Antipov wrote:
> When compiling with gcc version 14.0.0 20231129 (experimental) and
> CONFIG_FORTIFY_SOURCE=y, I've noticed the following:
> 
> In file included from ./include/linux/string.h:295,
>                  from ./include/linux/bitmap.h:12,
>                  from ./include/linux/cpumask.h:12,
>                  from ./arch/x86/include/asm/paravirt.h:17,
>                  from ./arch/x86/include/asm/irqflags.h:60,
>                  from ./include/linux/irqflags.h:17,
>                  from ./include/linux/rcupdate.h:26,
>                  from ./include/linux/rculist.h:11,
>                  from ./include/linux/pid.h:5,
>                  from ./include/linux/sched.h:14,
>                  from ./include/linux/ratelimit.h:6,
>                  from ./include/linux/dev_printk.h:16,
>                  from ./include/linux/device.h:15,

Not that relevant...

>                  from drivers/nfc/pn533/pn533.c:9:
> In function 'fortify_memcpy_chk',
>     inlined from 'pn533_target_found_felica' at drivers/nfc/pn533/pn533.c:781:2:
> ./include/linux/fortify-string.h:588:25: warning: call to '__read_overflow2_field'
> declared with attribute warning: detected read beyond size of field (2nd parameter);

This is unreadable. Please trim the logs to relevant parts preserving
formatting.

> maybe use struct_group()? [-Wattribute-warning]
>   588 |                         __read_overflow2_field(q_size_field, size);
> 
> Here the fortification logic interprets call to 'memcpy()' as an attempt
> to copy an amount of data which exceeds the size of the specified field
> (9 bytes from 1-byte 'opcode') and thus issues an overread warning -
> which is silenced by using the convenient 'struct_group()' quirk.


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

The subject PATCH should be with net-next, so it will be recognized by
net-dev patchwork.

Best regards,
Krzysztof