public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] nfc: pn533: Clear nfc_target before being used
@ 2022-12-13 14:27 Minsuk Kang
  2022-12-13 14:38 ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Minsuk Kang @ 2022-12-13 14:27 UTC (permalink / raw)
  To: krzysztof.kozlowski, netdev
  Cc: linma, davem, sameo, linville, dokyungs, jisoo.jang, Minsuk Kang

Fix a slab-out-of-bounds read that occurs in nla_put() called from
nfc_genl_send_target() when target->sensb_res_len, which is duplicated
from an nfc_target in pn533, is too large as the nfc_target is not
properly initialized and retains garbage values. Clear nfc_targets with
memset() before they are used.

Found by a modified version of syzkaller.

BUG: KASAN: slab-out-of-bounds in nla_put
Call Trace:
 memcpy
 nla_put
 nfc_genl_dump_targets
 genl_lock_dumpit
 netlink_dump
 __netlink_dump_start
 genl_family_rcv_msg_dumpit
 genl_rcv_msg
 netlink_rcv_skb
 genl_rcv
 netlink_unicast
 netlink_sendmsg
 sock_sendmsg
 ____sys_sendmsg
 ___sys_sendmsg
 __sys_sendmsg
 do_syscall_64

Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
Fixes: 361f3cb7f9cf ("NFC: DEP link hook implementation for pn533")
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
---
v1->v2:
  Clear another nfc_target in pn533_in_dep_link_up_complete()
  Fix the commit message

 drivers/nfc/pn533/pn533.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index d9f6367b9993..f0cac1900552 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -1295,6 +1295,8 @@ static int pn533_poll_dep_complete(struct pn533 *dev, void *arg,
 	if (IS_ERR(resp))
 		return PTR_ERR(resp);
 
+	memset(&nfc_target, 0, sizeof(struct nfc_target));
+
 	rsp = (struct pn533_cmd_jump_dep_response *)resp->data;
 
 	rc = rsp->status & PN533_CMD_RET_MASK;
@@ -1926,6 +1928,8 @@ static int pn533_in_dep_link_up_complete(struct pn533 *dev, void *arg,
 
 		dev_dbg(dev->dev, "Creating new target\n");
 
+		memset(&nfc_target, 0, sizeof(struct nfc_target));
+
 		nfc_target.supported_protocols = NFC_PROTO_NFC_DEP_MASK;
 		nfc_target.nfcid1_len = 10;
 		memcpy(nfc_target.nfcid1, rsp->nfcid3t, nfc_target.nfcid1_len);
-- 
2.25.1


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

* Re: [PATCH net v2] nfc: pn533: Clear nfc_target before being used
  2022-12-13 14:27 [PATCH net v2] nfc: pn533: Clear nfc_target before being used Minsuk Kang
@ 2022-12-13 14:38 ` Krzysztof Kozlowski
  2022-12-13 14:41   ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-13 14:38 UTC (permalink / raw)
  To: Minsuk Kang, netdev; +Cc: linma, davem, sameo, linville, dokyungs, jisoo.jang

On 13/12/2022 15:27, Minsuk Kang wrote:
> Fix a slab-out-of-bounds read that occurs in nla_put() called from
> nfc_genl_send_target() when target->sensb_res_len, which is duplicated
> from an nfc_target in pn533, is too large as the nfc_target is not
> properly initialized and retains garbage values. Clear nfc_targets with
> memset() before they are used.
> 
> Found by a modified version of syzkaller.
> 
> BUG: KASAN: slab-out-of-bounds in nla_put
> Call Trace:
>  memcpy
>  nla_put
>  nfc_genl_dump_targets
>  genl_lock_dumpit
>  netlink_dump
>  __netlink_dump_start
>  genl_family_rcv_msg_dumpit
>  genl_rcv_msg
>  netlink_rcv_skb
>  genl_rcv
>  netlink_unicast
>  netlink_sendmsg
>  sock_sendmsg
>  ____sys_sendmsg
>  ___sys_sendmsg
>  __sys_sendmsg
>  do_syscall_64
> 
> Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
> Fixes: 361f3cb7f9cf ("NFC: DEP link hook implementation for pn533")
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

How did it happen? From where did you get it?

> Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
> ---
> v1->v2:
>   Clear another nfc_target in pn533_in_dep_link_up_complete()
>   Fix the commit message
> 


Best regards,
Krzysztof


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

* Re: [PATCH net v2] nfc: pn533: Clear nfc_target before being used
  2022-12-13 14:38 ` Krzysztof Kozlowski
@ 2022-12-13 14:41   ` Krzysztof Kozlowski
  2022-12-13 16:03     ` Minsuk Kang
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-13 14:41 UTC (permalink / raw)
  To: Minsuk Kang, netdev; +Cc: linma, davem, sameo, linville, dokyungs, jisoo.jang

On 13/12/2022 15:38, Krzysztof Kozlowski wrote:
> On 13/12/2022 15:27, Minsuk Kang wrote:
>> Fix a slab-out-of-bounds read that occurs in nla_put() called from
>> nfc_genl_send_target() when target->sensb_res_len, which is duplicated
>> from an nfc_target in pn533, is too large as the nfc_target is not
>> properly initialized and retains garbage values. Clear nfc_targets with
>> memset() before they are used.
>>
>> Found by a modified version of syzkaller.
>>
>> BUG: KASAN: slab-out-of-bounds in nla_put
>> Call Trace:
>>  memcpy
>>  nla_put
>>  nfc_genl_dump_targets
>>  genl_lock_dumpit
>>  netlink_dump
>>  __netlink_dump_start
>>  genl_family_rcv_msg_dumpit
>>  genl_rcv_msg
>>  netlink_rcv_skb
>>  genl_rcv
>>  netlink_unicast
>>  netlink_sendmsg
>>  sock_sendmsg
>>  ____sys_sendmsg
>>  ___sys_sendmsg
>>  __sys_sendmsg
>>  do_syscall_64
>>
>> Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
>> Fixes: 361f3cb7f9cf ("NFC: DEP link hook implementation for pn533")
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> How did it happen? From where did you get it?

I double checked - I did not send it. This is some fake tag. Please do
not add fake/invented/created tags with people's names.

Best regards,
Krzysztof


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

* Re: Re: [PATCH net v2] nfc: pn533: Clear nfc_target before being used
  2022-12-13 14:41   ` Krzysztof Kozlowski
@ 2022-12-13 16:03     ` Minsuk Kang
  2022-12-13 16:05       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Minsuk Kang @ 2022-12-13 16:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski, netdev
  Cc: linma, davem, sameo, linville, dokyungs, jisoo.jang, Minsuk Kang

On Tue, Dec 13, 2022 at 03:41:36PM +0100, Krzysztof Kozlowski wrote:
> On 13/12/2022 15:38, Krzysztof Kozlowski wrote:
> > On 13/12/2022 15:27, Minsuk Kang wrote:
> >> Fix a slab-out-of-bounds read that occurs in nla_put() called from
> >> nfc_genl_send_target() when target->sensb_res_len, which is duplicated
> >> from an nfc_target in pn533, is too large as the nfc_target is not
> >> properly initialized and retains garbage values. Clear nfc_targets with
> >> memset() before they are used.
> >>
> >> Found by a modified version of syzkaller.
> >>
> >> BUG: KASAN: slab-out-of-bounds in nla_put
> >> Call Trace:
> >>  memcpy
> >>  nla_put
> >>  nfc_genl_dump_targets
> >>  genl_lock_dumpit
> >>  netlink_dump
> >>  __netlink_dump_start
> >>  genl_family_rcv_msg_dumpit
> >>  genl_rcv_msg
> >>  netlink_rcv_skb
> >>  genl_rcv
> >>  netlink_unicast
> >>  netlink_sendmsg
> >>  sock_sendmsg
> >>  ____sys_sendmsg
> >>  ___sys_sendmsg
> >>  __sys_sendmsg
> >>  do_syscall_64
> >>
> >> Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
> >> Fixes: 361f3cb7f9cf ("NFC: DEP link hook implementation for pn533")
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > 
> > How did it happen? From where did you get it?
> 
> I double checked - I did not send it. This is some fake tag. Please do
> not add fake/invented/created tags with people's names.

Sorry for my confusion.

https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L505

I missed the definition of the tag as I did not read the document
carefully and misunderstood that the tag simply means I have got a
reply from maintainers and I should manually attach it if that is
the case. I will rewrite the patch after I make sure I fully
understand the whole rules.

Best regards,
Minsuk

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

* Re: [PATCH net v2] nfc: pn533: Clear nfc_target before being used
  2022-12-13 16:03     ` Minsuk Kang
@ 2022-12-13 16:05       ` Krzysztof Kozlowski
  2022-12-13 16:22         ` Minsuk Kang
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-13 16:05 UTC (permalink / raw)
  To: Minsuk Kang, netdev; +Cc: linma, davem, sameo, linville, dokyungs, jisoo.jang

On 13/12/2022 17:03, Minsuk Kang wrote:
> On Tue, Dec 13, 2022 at 03:41:36PM +0100, Krzysztof Kozlowski wrote:
>> On 13/12/2022 15:38, Krzysztof Kozlowski wrote:
>>> On 13/12/2022 15:27, Minsuk Kang wrote:
>>>> Fix a slab-out-of-bounds read that occurs in nla_put() called from
>>>> nfc_genl_send_target() when target->sensb_res_len, which is duplicated
>>>> from an nfc_target in pn533, is too large as the nfc_target is not
>>>> properly initialized and retains garbage values. Clear nfc_targets with
>>>> memset() before they are used.
>>>>
>>>> Found by a modified version of syzkaller.
>>>>
>>>> BUG: KASAN: slab-out-of-bounds in nla_put
>>>> Call Trace:
>>>>  memcpy
>>>>  nla_put
>>>>  nfc_genl_dump_targets
>>>>  genl_lock_dumpit
>>>>  netlink_dump
>>>>  __netlink_dump_start
>>>>  genl_family_rcv_msg_dumpit
>>>>  genl_rcv_msg
>>>>  netlink_rcv_skb
>>>>  genl_rcv
>>>>  netlink_unicast
>>>>  netlink_sendmsg
>>>>  sock_sendmsg
>>>>  ____sys_sendmsg
>>>>  ___sys_sendmsg
>>>>  __sys_sendmsg
>>>>  do_syscall_64
>>>>
>>>> Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
>>>> Fixes: 361f3cb7f9cf ("NFC: DEP link hook implementation for pn533")
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>
>>> How did it happen? From where did you get it?
>>
>> I double checked - I did not send it. This is some fake tag. Please do
>> not add fake/invented/created tags with people's names.
> 
> Sorry for my confusion.
> 
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L505
> 
> I missed the definition of the tag as I did not read the document
> carefully and misunderstood that the tag simply means I have got a
> reply from maintainers and I should manually attach it if that is
> the case. I will rewrite the patch after I make sure I fully
> understand the whole rules.

The document says:
"By offering my Reviewed-by: tag, I state that:"

You need to receive it explicitly from the reviewer. Once received, but
only then, add to the patch.

Best regards,
Krzysztof


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

* Re: [PATCH net v2] nfc: pn533: Clear nfc_target before being used
  2022-12-13 16:05       ` Krzysztof Kozlowski
@ 2022-12-13 16:22         ` Minsuk Kang
  0 siblings, 0 replies; 6+ messages in thread
From: Minsuk Kang @ 2022-12-13 16:22 UTC (permalink / raw)
  To: Krzysztof Kozlowski, netdev
  Cc: linma, davem, sameo, linville, dokyungs, jisoo.jang, Minsuk Kang

On Tue, Dec 13, 2022 at 05:05:27PM +0100, Krzysztof Kozlowski wrote:
> On 13/12/2022 17:03, Minsuk Kang wrote:
> > On Tue, Dec 13, 2022 at 03:41:36PM +0100, Krzysztof Kozlowski wrote:
> >> On 13/12/2022 15:38, Krzysztof Kozlowski wrote:
> >>> On 13/12/2022 15:27, Minsuk Kang wrote:
> >>>> Fix a slab-out-of-bounds read that occurs in nla_put() called from
> >>>> nfc_genl_send_target() when target->sensb_res_len, which is duplicated
> >>>> from an nfc_target in pn533, is too large as the nfc_target is not
> >>>> properly initialized and retains garbage values. Clear nfc_targets with
> >>>> memset() before they are used.
> >>>>
> >>>> Found by a modified version of syzkaller.
> >>>>
> >>>> BUG: KASAN: slab-out-of-bounds in nla_put
> >>>> Call Trace:
> >>>>  memcpy
> >>>>  nla_put
> >>>>  nfc_genl_dump_targets
> >>>>  genl_lock_dumpit
> >>>>  netlink_dump
> >>>>  __netlink_dump_start
> >>>>  genl_family_rcv_msg_dumpit
> >>>>  genl_rcv_msg
> >>>>  netlink_rcv_skb
> >>>>  genl_rcv
> >>>>  netlink_unicast
> >>>>  netlink_sendmsg
> >>>>  sock_sendmsg
> >>>>  ____sys_sendmsg
> >>>>  ___sys_sendmsg
> >>>>  __sys_sendmsg
> >>>>  do_syscall_64
> >>>>
> >>>> Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
> >>>> Fixes: 361f3cb7f9cf ("NFC: DEP link hook implementation for pn533")
> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>
> >>> How did it happen? From where did you get it?
> >>
> >> I double checked - I did not send it. This is some fake tag. Please do
> >> not add fake/invented/created tags with people's names.
> > 
> > Sorry for my confusion.
> > 
> > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L505
> > 
> > I missed the definition of the tag as I did not read the document
> > carefully and misunderstood that the tag simply means I have got a
> > reply from maintainers and I should manually attach it if that is
> > the case. I will rewrite the patch after I make sure I fully
> > understand the whole rules.
> 
> The document says:
> "By offering my Reviewed-by: tag, I state that:"
> 
> You need to receive it explicitly from the reviewer. Once received, but
> only then, add to the patch.

Thank you for your comment.
I won't forget it.

Best regards,
Minsuk

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

end of thread, other threads:[~2022-12-13 16:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-13 14:27 [PATCH net v2] nfc: pn533: Clear nfc_target before being used Minsuk Kang
2022-12-13 14:38 ` Krzysztof Kozlowski
2022-12-13 14:41   ` Krzysztof Kozlowski
2022-12-13 16:03     ` Minsuk Kang
2022-12-13 16:05       ` Krzysztof Kozlowski
2022-12-13 16:22         ` Minsuk Kang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox