Re: [PATCH] nfc: pn533: Add poll mod list filling check

From: Aleksandr Mishin
Date: Wed Jul 03 2024 - 03:32:29 EST




On 03.07.2024 8:02, Krzysztof Kozlowski wrote:
On 02/07/2024 11:39, Aleksandr Mishin wrote:
In case of im_protocols value is 1 and tm_protocols value is 0 this

Which im protocol has value 1 in the mask?

The pn533_poll_create_mod_list() handles all possible masks, so your
case is just not possible to happen.

Exactly. pn533_poll_create_mod_list() handles all possible specified masks. No im protocol has value 1 in the mask. In case of 'im_protocol' parameter has value of 1, no mod will be added. So dev->poll_mod_count will remain 0.
I assume 'im_protocol' parameter is "external" to this driver, it comes from outside and can contain any value, so driver has to be able to protect itself from incorrect values.


This patch is purely to satisfy (your) static analyzers, so this should
be clear in commit msg. You are not fixing any bug but adding sort of
defensive code and suppresion of false-positive warning...

combination successfully passes the check
'if (!im_protocols && !tm_protocols)' in the nfc_start_poll().
But then after pn533_poll_create_mod_list() call in pn533_start_poll()
poll mod list will remain empty and dev->poll_mod_count will remain 0
which lead to division by zero.

Add poll mod list filling check.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: dfccd0f58044 ("NFC: pn533: Add some polling entropy")
Signed-off-by: Aleksandr Mishin <amishin@xxxxxxxxxx>
---
drivers/nfc/pn533/pn533.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index b19c39dcfbd9..e2bc67300a91 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -1723,6 +1723,11 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
}
pn533_poll_create_mod_list(dev, im_protocols, tm_protocols);
+ if (!dev->poll_mod_count) {
+ nfc_err(dev->dev,
+ "Poll mod list is empty\n");

Odd wrapping.

Just like other messages in this function.


+ return -EINVAL;
+ }
/* Do not always start polling from the same modulation */
get_random_bytes(&rand_mod, sizeof(rand_mod));

Best regards,
Krzysztof


--
Kind regards
Aleksandr