* [PATCH net] nfc: pn533: Clear nfc_target in pn533_poll_dep_complete() before being used
@ 2022-12-13 1:41 Minsuk Kang
2022-12-13 10:45 ` Krzysztof Kozlowski
0 siblings, 1 reply; 4+ messages in thread
From: Minsuk Kang @ 2022-12-13 1:41 UTC (permalink / raw)
To: krzysztof.kozlowski, netdev
Cc: linma, davem, sameo, dokyungs, jisoo.jang, Minsuk Kang
This patch fixes a slab-out-of-bounds read in pn533 that occurs in
nla_put() called from nfc_genl_send_target() when target->sensb_res_len,
which is duplicated from nfc_target in pn533_poll_dep_complete(), is
too large as the nfc_target is not properly initialized and retains
garbage values. The patch clears the nfc_target before it is used.
Found by a modified version of syzkaller.
==================================================================
BUG: KASAN: slab-out-of-bounds in nla_put+0xe0/0x120
Read of size 94 at addr ffff888109d1dfa0 by task syz-executor/4367
CPU: 0 PID: 4367 Comm: syz-executor Not tainted 5.14.0+ #171
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
Call Trace:
dump_stack_lvl+0x8e/0xd1
print_address_description.constprop.0.cold+0x93/0x334
kasan_report.cold+0x83/0xdf
kasan_check_range+0x14e/0x1b0
memcpy+0x20/0x60
nla_put+0xe0/0x120
nfc_genl_dump_targets+0x74f/0xb20
genl_lock_dumpit+0x65/0x90
netlink_dump+0x4b0/0xa40
__netlink_dump_start+0x5dc/0x8c0
genl_family_rcv_msg_dumpit.isra.0+0x2a1/0x300
genl_rcv_msg+0x3c8/0x4f0
netlink_rcv_skb+0x130/0x3b0
genl_rcv+0x29/0x40
netlink_unicast+0x4a1/0x6a0
netlink_sendmsg+0x788/0xc90
sock_sendmsg+0xca/0x110
____sys_sendmsg+0x63f/0x780
___sys_sendmsg+0xfb/0x170
__sys_sendmsg+0xd8/0x190
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x46b55d
Code: 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f167a757c48 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000686b60 RCX: 000000000046b55d
RDX: 0000000000000000 RSI: 000000004004fb80 RDI: 0000000000000009
RBP: 00000000004d9ba0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000686b60
R13: 0000000000686b68 R14: 00007ffd2f2facc0 R15: 00007f167a757dc0
Allocated by task 0:
(stack is not available)
The buggy address belongs to the object at ffff888109d1df80
which belongs to the cache kmalloc-96 of size 96
The buggy address is located 32 bytes inside of
96-byte region [ffff888109d1df80, ffff888109d1dfe0)
The buggy address belongs to the page:
page:ffffea0004274740 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x109d1d
flags: 0x200000000000200(slab|node=0|zone=2)
raw: 0200000000000200 0000000000000000 dead000000000122 ffff888100041780
raw: 0000000000000000 0000000080200020 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12c40(GFP_NOFS|__GFP_NOWARN|__GFP_NORETRY), pid 4366, ts 19572546791, free_ts 19568585127
prep_new_page+0x1aa/0x240
get_page_from_freelist+0x159a/0x27c0
__alloc_pages+0x2da/0x6a0
alloc_pages+0xec/0x1e0
allocate_slab+0x380/0x4e0
___slab_alloc+0x5bc/0x940
__slab_alloc+0x6d/0x80
__kmalloc+0x329/0x390
tomoyo_supervisor+0xb7f/0xd10
tomoyo_path_permission+0x26a/0x3a0
tomoyo_check_open_permission+0x2b0/0x310
tomoyo_file_open+0x99/0xc0
security_file_open+0x57/0x470
do_dentry_open+0x318/0xfe0
path_openat+0x1852/0x2310
do_filp_open+0x1c6/0x290
page last free stack trace:
free_pcp_prepare+0x3d3/0x7f0
free_unref_page+0x1e/0x3d0
unfreeze_partials.isra.0+0x211/0x2f0
put_cpu_partial+0x66/0x160
qlist_free_all+0x5a/0xc0
kasan_quarantine_reduce+0x13d/0x180
__kasan_slab_alloc+0x73/0x80
slab_post_alloc_hook+0x4d/0x490
__kmalloc+0x180/0x390
tomoyo_supervisor+0xb7f/0xd10
tomoyo_path_permission+0x26a/0x3a0
tomoyo_path_perm+0x2c1/0x3c0
security_inode_getattr+0xc2/0x130
vfs_getattr+0x27/0x60
vfs_fstat+0x3e/0x80
__do_sys_newfstat+0x7d/0xf0
Memory state around the buggy address:
ffff888109d1de80: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
ffff888109d1df00: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
>ffff888109d1df80: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
^
ffff888109d1e000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff888109d1e080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================
Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr>
Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr>
Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
---
drivers/nfc/pn533/pn533.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index d9f6367b9993..c6a611622668 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;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] nfc: pn533: Clear nfc_target in pn533_poll_dep_complete() before being used
2022-12-13 1:41 [PATCH net] nfc: pn533: Clear nfc_target in pn533_poll_dep_complete() before being used Minsuk Kang
@ 2022-12-13 10:45 ` Krzysztof Kozlowski
2022-12-13 14:20 ` Minsuk Kang
0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-13 10:45 UTC (permalink / raw)
To: Minsuk Kang, netdev; +Cc: linma, davem, sameo, dokyungs, jisoo.jang
On 13/12/2022 02:41, Minsuk Kang wrote:
> This patch fixes a slab-out-of-bounds read in pn533 that occurs in
Do not use "This commit/patch".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> nla_put() called from nfc_genl_send_target() when target->sensb_res_len,
> which is duplicated from nfc_target in pn533_poll_dep_complete(), is
> too large as the nfc_target is not properly initialized and retains
> garbage values. The patch clears the nfc_target before it is used.
Same here
>
> Found by a modified version of syzkaller.
>
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in nla_put+0xe0/0x120
> Read of size 94 at addr ffff888109d1dfa0 by task syz-executor/4367
>
> CPU: 0 PID: 4367 Comm: syz-executor Not tainted 5.14.0+ #171
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> Call Trace:
> dump_stack_lvl+0x8e/0xd1
> print_address_description.constprop.0.cold+0x93/0x334
> kasan_report.cold+0x83/0xdf
> kasan_check_range+0x14e/0x1b0
> memcpy+0x20/0x60
> nla_put+0xe0/0x120
> nfc_genl_dump_targets+0x74f/0xb20
> genl_lock_dumpit+0x65/0x90
> netlink_dump+0x4b0/0xa40
> __netlink_dump_start+0x5dc/0x8c0
> genl_family_rcv_msg_dumpit.isra.0+0x2a1/0x300
> genl_rcv_msg+0x3c8/0x4f0
> netlink_rcv_skb+0x130/0x3b0
> genl_rcv+0x29/0x40
> netlink_unicast+0x4a1/0x6a0
> netlink_sendmsg+0x788/0xc90
> sock_sendmsg+0xca/0x110
> ____sys_sendmsg+0x63f/0x780
> ___sys_sendmsg+0xfb/0x170
> __sys_sendmsg+0xd8/0x190
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x46b55d
> Code: 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007f167a757c48 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> RAX: ffffffffffffffda RBX: 0000000000686b60 RCX: 000000000046b55d
> RDX: 0000000000000000 RSI: 000000004004fb80 RDI: 0000000000000009
> RBP: 00000000004d9ba0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000686b60
> R13: 0000000000686b68 R14: 00007ffd2f2facc0 R15: 00007f167a757dc0
>
> Allocated by task 0:
> (stack is not available)
>
> The buggy address belongs to the object at ffff888109d1df80
> which belongs to the cache kmalloc-96 of size 96
> The buggy address is located 32 bytes inside of
> 96-byte region [ffff888109d1df80, ffff888109d1dfe0)
> The buggy address belongs to the page:
> page:ffffea0004274740 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x109d1d
> flags: 0x200000000000200(slab|node=0|zone=2)
> raw: 0200000000000200 0000000000000000 dead000000000122 ffff888100041780
> raw: 0000000000000000 0000000080200020 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
> page_owner tracks the page as allocated
> page last allocated via order 0, migratetype Unmovable, gfp_mask 0x12c40(GFP_NOFS|__GFP_NOWARN|__GFP_NORETRY), pid 4366, ts 19572546791, free_ts 19568585127
> prep_new_page+0x1aa/0x240
> get_page_from_freelist+0x159a/0x27c0
> __alloc_pages+0x2da/0x6a0
> alloc_pages+0xec/0x1e0
> allocate_slab+0x380/0x4e0
> ___slab_alloc+0x5bc/0x940
> __slab_alloc+0x6d/0x80
> __kmalloc+0x329/0x390
> tomoyo_supervisor+0xb7f/0xd10
> tomoyo_path_permission+0x26a/0x3a0
> tomoyo_check_open_permission+0x2b0/0x310
> tomoyo_file_open+0x99/0xc0
> security_file_open+0x57/0x470
> do_dentry_open+0x318/0xfe0
> path_openat+0x1852/0x2310
> do_filp_open+0x1c6/0x290
> page last free stack trace:
> free_pcp_prepare+0x3d3/0x7f0
> free_unref_page+0x1e/0x3d0
> unfreeze_partials.isra.0+0x211/0x2f0
> put_cpu_partial+0x66/0x160
> qlist_free_all+0x5a/0xc0
> kasan_quarantine_reduce+0x13d/0x180
> __kasan_slab_alloc+0x73/0x80
> slab_post_alloc_hook+0x4d/0x490
> __kmalloc+0x180/0x390
> tomoyo_supervisor+0xb7f/0xd10
> tomoyo_path_permission+0x26a/0x3a0
> tomoyo_path_perm+0x2c1/0x3c0
> security_inode_getattr+0xc2/0x130
> vfs_getattr+0x27/0x60
> vfs_fstat+0x3e/0x80
> __do_sys_newfstat+0x7d/0xf0
>
> Memory state around the buggy address:
> ffff888109d1de80: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> ffff888109d1df00: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
>> ffff888109d1df80: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
> ^
> ffff888109d1e000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff888109d1e080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Drop unrelated pieces of OOPS and keep only things which are relevant.
> ==================================================================
>
> Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
> Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr>
> Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr>
> Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
Reported-by is for crediting other people, not crediting yourself.
Otherwise all my patches would be reported-by, right? Please drop this
one and keep only credit for other people who actually reported it. It's
anyway weird to see three people reporting one bug.
Additionally I really dislike private reports because they sometimes
cannot be trusted (see all the fake report credits from running
coccinelle by Hulk Robot and others)... Care to provide link to the
reports of this bug?
> Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
> ---
> drivers/nfc/pn533/pn533.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
> index d9f6367b9993..c6a611622668 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));
There is one more place to fix in pn533_in_dep_link_up_complete()
> +
> rsp = (struct pn533_cmd_jump_dep_response *)resp->data;
>
> rc = rsp->status & PN533_CMD_RET_MASK;
> --
> 2.25.1
>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH net] nfc: pn533: Clear nfc_target in pn533_poll_dep_complete() before being used
2022-12-13 10:45 ` Krzysztof Kozlowski
@ 2022-12-13 14:20 ` Minsuk Kang
2022-12-13 14:40 ` Krzysztof Kozlowski
0 siblings, 1 reply; 4+ messages in thread
From: Minsuk Kang @ 2022-12-13 14:20 UTC (permalink / raw)
To: Krzysztof Kozlowski, netdev
Cc: linma, davem, sameo, dokyungs, jisoo.jang, Minsuk Kang
On Tue, Dec 13, 2022 at 11:45:53AM +0100, Krzysztof Kozlowski wrote:
> > This patch fixes a slab-out-of-bounds read in pn533 that occurs in
>
> Do not use "This commit/patch".
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
> > nla_put() called from nfc_genl_send_target() when target->sensb_res_len,
> > which is duplicated from nfc_target in pn533_poll_dep_complete(), is
> > too large as the nfc_target is not properly initialized and retains
> > garbage values. The patch clears the nfc_target before it is used.
>
> Same here
>
> >
> > Found by a modified version of syzkaller.
> >
> > ==================================================================
> > BUG: KASAN: slab-out-of-bounds in nla_put+0xe0/0x120
> > Read of size 94 at addr ffff888109d1dfa0 by task syz-executor/4367
> >
> > CPU: 0 PID: 4367 Comm: syz-executor Not tainted 5.14.0+ #171
[snip]
> > Memory state around the buggy address:
> > ffff888109d1de80: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > ffff888109d1df00: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
> >> ffff888109d1df80: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
> > ^
> > ffff888109d1e000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ffff888109d1e080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>
> Drop unrelated pieces of OOPS and keep only things which are relevant.
>
Thank you for the comments. I will update the commit message as advised
in v2.
> > ==================================================================
> >
> > Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
> > Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr>
> > Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr>
> > Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
>
> Reported-by is for crediting other people, not crediting yourself.
> Otherwise all my patches would be reported-by, right? Please drop this
> one and keep only credit for other people who actually reported it. It's
> anyway weird to see three people reporting one bug.
>
> Additionally I really dislike private reports because they sometimes
> cannot be trusted (see all the fake report credits from running
> coccinelle by Hulk Robot and others)... Care to provide link to the
> reports of this bug?
>
My intention was to credit all the people contributed to the
modification of syzkaller that led to this bug. But I will drop them in
v2.
> > Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
> > ---
> > drivers/nfc/pn533/pn533.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
> > index d9f6367b9993..c6a611622668 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));
>
> There is one more place to fix in pn533_in_dep_link_up_complete()
Thank you. I will add a fix for it in v2.
Best regards,
Minsuk
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] nfc: pn533: Clear nfc_target in pn533_poll_dep_complete() before being used
2022-12-13 14:20 ` Minsuk Kang
@ 2022-12-13 14:40 ` Krzysztof Kozlowski
0 siblings, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-13 14:40 UTC (permalink / raw)
To: Minsuk Kang, netdev; +Cc: linma, davem, sameo, dokyungs, jisoo.jang
On 13/12/2022 15:20, Minsuk Kang wrote:
>
>>> ==================================================================
>>>
>>> Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
>>> Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr>
>>> Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr>
>>> Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
>>
>> Reported-by is for crediting other people, not crediting yourself.
>> Otherwise all my patches would be reported-by, right? Please drop this
>> one and keep only credit for other people who actually reported it. It's
>> anyway weird to see three people reporting one bug.
>>
>> Additionally I really dislike private reports because they sometimes
>> cannot be trusted (see all the fake report credits from running
>> coccinelle by Hulk Robot and others)... Care to provide link to the
>> reports of this bug?
>>
>
> My intention was to credit all the people contributed to the
> modification of syzkaller that led to this bug. But I will drop them in
> v2.
Then shouldn't you also credit all authors of original syzkaller as
well? And people who wrote core libraries being used there? Let's don't
go that way...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-12-13 14:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-13 1:41 [PATCH net] nfc: pn533: Clear nfc_target in pn533_poll_dep_complete() before being used Minsuk Kang
2022-12-13 10:45 ` Krzysztof Kozlowski
2022-12-13 14:20 ` Minsuk Kang
2022-12-13 14:40 ` Krzysztof Kozlowski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.