* [PATCH] To fix the below failure of handling page fault caused by the invalid input from user. @ 2022-01-18 9:19 Dongyang Wang 2022-01-26 2:42 ` Andrew Morton 0 siblings, 1 reply; 7+ messages in thread From: Dongyang Wang @ 2022-01-18 9:19 UTC (permalink / raw) To: christian.brauner, akpm, ebiederm, legion, varad.gautam; +Cc: linux-kernel [786058.308965] Unable to handle kernel paging request at virtual address 01000004 [786058.316286] pgd = 38a99693 [786058.319080] [01000004] *pgd=07800003, *pmd=00000000 [786058.324056] Internal error: Oops: 206 [#1] PREEMPT SMP ARM [786058.324100] CPU: PID: Comm: Tainted: G C [786058.324102] Hardware name: [786058.324114] PC is at __copy_to_user_std+0x4c/0x3c4 [786058.324120] LR is at store_msg+0xc0/0xe8 [786058.324124] pc : [<c0c0587c>] lr : [<c0871d04>] psr: 20010013 [786058.324126] sp : c3503ec4 ip : 00000000 fp : b4c9a660 [786058.324129] r10: c4228dc0 r9 : c3502000 r8 : 00000ffc [786058.324132] r7 : 01000000 r6 : 546d3f8b r5 : b4911690 r4 : 00000ffc [786058.324134] r3 : 00000000 r2 : 00000f7c r1 : 01000004 r0 : b4911690 [786058.324139] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user [786058.324142] Control: 30c5387d Table: 0edc2040 DAC: 55555555 [786058.324145] Process (pid: , stack limit = 0x25018bdf) [786058.324148] Stack: (0xc3503ec4 to 0xc3504000) [786058.324153] 3ec0: b4911690 546d3f8b 01000000 00000ffc b4911690 00000ffc 00000000 [786058.324157] 3ee0: 00000ffc c0871d04 546d4f73 c3407801 c3503f28 c3407800 00000000 b49106a8 [786058.324161] 3f00: c4228dc0 c087abd4 00000002 b49106a8 617b9d03 00000000 00000000 c121d508 [786058.324165] 3f20: 00000000 bf06a1a8 d1b634cc 16b26e77 c5af5280 00000100 00000200 db806540 [786058.324170] 3f40: 00000001 c121d508 00000008 0000005c 00000000 00010008 b49106a8 c0601208 [786058.324173] 3f60: c3502000 00000040 b4c9a660 c087b474 c3503f78 c121d508 617b9d03 00000000 [786058.324177] 3f80: 2303d6cc 00000115 c0601208 c121d508 b4c9a660 b4c9a660 00000001 b49106a8 [786058.324181] 3fa0: 00000115 c06011dc b4c9a660 00000001 0000005c b49106a8 00010008 00000000 [786058.324185] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 [786058.324189] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 800d0030 0000005c 00000000 00000000 [786058.324201] [<c0c0587c>] (__copy_to_user_std) from [<c0871d04>] (store_msg+0xc0/0xe8) [786058.324211] [<c0871d04>] (store_msg) from [<c087abd4>] (do_mq_timedreceive+0x29c/0x484) [786058.324218] [<c087abd4>] (do_mq_timedreceive) from [<c087b474>] (sys_mq_timedreceive+0x88/0xbc) [786058.324226] [<c087b474>] (sys_mq_timedreceive) from [<c06011dc>] (__sys_trace_return+0x0/0x10) [786058.324229] Exception stack(0xc3503fa8 to 0xc3503ff0) [786058.324233] 3fa0: b4c9a660 00000001 0000005c b49106a8 00010008 00000000 [786058.324236] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 [786058.324239] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 [786058.324247] Code: ba000002 f5d1f03c f5d1f05c f5d1f07c (e8b151f8) [786058.601808] ---[ end trace 0000000000000002 ]--- Signed-off-by: Fuchun Sun <fuchun.sun@windriver.com> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> Signed-off-by: Jing Zhang <Jing.Zhang@windriver.com> Signed-off-by: Dongyang Wang <dongyang.wang@windriver.com> --- ipc/mqueue.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ipc/mqueue.c b/ipc/mqueue.c index 5becca9be867c..c904400f8fd9f 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -1211,6 +1211,12 @@ static int do_mq_timedreceive(mqd_t mqdes, char __user *u_msg_ptr, goto out_fput; } + /* checks if buffer is invalid */ + if ((unsigned long) u_msg_ptr < PAGE_SIZE) { + ret = -EINVAL; + goto out_fput; + } + /* * msg_insert really wants us to have a valid, spare node struct so * it doesn't have to kmalloc a GFP_ATOMIC allocation, but it will @@ -1262,6 +1268,10 @@ static int do_mq_timedreceive(mqd_t mqdes, char __user *u_msg_ptr, store_msg(u_msg_ptr, msg_ptr, msg_ptr->m_ts)) { ret = -EFAULT; } + + if (msg_len < ret) + ret = -EMSGSIZE; + free_msg(msg_ptr); } out_fput: -- 2.26.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] To fix the below failure of handling page fault caused by the invalid input from user. 2022-01-18 9:19 [PATCH] To fix the below failure of handling page fault caused by the invalid input from user Dongyang Wang @ 2022-01-26 2:42 ` Andrew Morton 2022-01-26 16:52 ` Manfred Spraul 0 siblings, 1 reply; 7+ messages in thread From: Andrew Morton @ 2022-01-26 2:42 UTC (permalink / raw) To: Dongyang Wang Cc: christian.brauner, ebiederm, legion, varad.gautam, linux-kernel, Manfred Spraul, Davidlohr Bueso On Tue, 18 Jan 2022 17:19:52 +0800 Dongyang Wang <dongyang.wang@windriver.com> wrote: > [786058.308965] Unable to handle kernel paging request at virtual address 01000004 > [786058.316286] pgd = 38a99693 > [786058.319080] [01000004] *pgd=07800003, *pmd=00000000 > [786058.324056] Internal error: Oops: 206 [#1] PREEMPT SMP ARM > [786058.324100] CPU: PID: Comm: Tainted: G C > [786058.324102] Hardware name: > [786058.324114] PC is at __copy_to_user_std+0x4c/0x3c4 > [786058.324120] LR is at store_msg+0xc0/0xe8 > [786058.324124] pc : [<c0c0587c>] lr : [<c0871d04>] psr: 20010013 > [786058.324126] sp : c3503ec4 ip : 00000000 fp : b4c9a660 > [786058.324129] r10: c4228dc0 r9 : c3502000 r8 : 00000ffc > [786058.324132] r7 : 01000000 r6 : 546d3f8b r5 : b4911690 r4 : 00000ffc > [786058.324134] r3 : 00000000 r2 : 00000f7c r1 : 01000004 r0 : b4911690 > [786058.324139] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user > [786058.324142] Control: 30c5387d Table: 0edc2040 DAC: 55555555 > [786058.324145] Process (pid: , stack limit = 0x25018bdf) > [786058.324148] Stack: (0xc3503ec4 to 0xc3504000) > [786058.324153] 3ec0: b4911690 546d3f8b 01000000 00000ffc b4911690 00000ffc 00000000 > [786058.324157] 3ee0: 00000ffc c0871d04 546d4f73 c3407801 c3503f28 c3407800 00000000 b49106a8 > [786058.324161] 3f00: c4228dc0 c087abd4 00000002 b49106a8 617b9d03 00000000 00000000 c121d508 > [786058.324165] 3f20: 00000000 bf06a1a8 d1b634cc 16b26e77 c5af5280 00000100 00000200 db806540 > [786058.324170] 3f40: 00000001 c121d508 00000008 0000005c 00000000 00010008 b49106a8 c0601208 > [786058.324173] 3f60: c3502000 00000040 b4c9a660 c087b474 c3503f78 c121d508 617b9d03 00000000 > [786058.324177] 3f80: 2303d6cc 00000115 c0601208 c121d508 b4c9a660 b4c9a660 00000001 b49106a8 > [786058.324181] 3fa0: 00000115 c06011dc b4c9a660 00000001 0000005c b49106a8 00010008 00000000 > [786058.324185] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 > [786058.324189] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 800d0030 0000005c 00000000 00000000 > [786058.324201] [<c0c0587c>] (__copy_to_user_std) from [<c0871d04>] (store_msg+0xc0/0xe8) > [786058.324211] [<c0871d04>] (store_msg) from [<c087abd4>] (do_mq_timedreceive+0x29c/0x484) > [786058.324218] [<c087abd4>] (do_mq_timedreceive) from [<c087b474>] (sys_mq_timedreceive+0x88/0xbc) > [786058.324226] [<c087b474>] (sys_mq_timedreceive) from [<c06011dc>] (__sys_trace_return+0x0/0x10) > [786058.324229] Exception stack(0xc3503fa8 to 0xc3503ff0) > [786058.324233] 3fa0: b4c9a660 00000001 0000005c b49106a8 00010008 00000000 > [786058.324236] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 > [786058.324239] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 > [786058.324247] Code: ba000002 f5d1f03c f5d1f05c f5d1f07c (e8b151f8) > [786058.601808] ---[ end trace 0000000000000002 ]--- > > ... > Please describe the circumstances under which this occurs. Please also describe how your proposed patch fixes this. > --- a/ipc/mqueue.c > +++ b/ipc/mqueue.c > @@ -1211,6 +1211,12 @@ static int do_mq_timedreceive(mqd_t mqdes, char __user *u_msg_ptr, > goto out_fput; > } > > + /* checks if buffer is invalid */ > + if ((unsigned long) u_msg_ptr < PAGE_SIZE) { > + ret = -EINVAL; > + goto out_fput; > + } > + > /* > * msg_insert really wants us to have a valid, spare node struct so > * it doesn't have to kmalloc a GFP_ATOMIC allocation, but it will > @@ -1262,6 +1268,10 @@ static int do_mq_timedreceive(mqd_t mqdes, char __user *u_msg_ptr, > store_msg(u_msg_ptr, msg_ptr, msg_ptr->m_ts)) { > ret = -EFAULT; > } > + > + if (msg_len < ret) > + ret = -EMSGSIZE; > + > free_msg(msg_ptr); > } > out_fput: > -- > 2.26.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] To fix the below failure of handling page fault caused by the invalid input from user. 2022-01-26 2:42 ` Andrew Morton @ 2022-01-26 16:52 ` Manfred Spraul 2022-01-29 15:02 ` Dongyang Wang 0 siblings, 1 reply; 7+ messages in thread From: Manfred Spraul @ 2022-01-26 16:52 UTC (permalink / raw) To: Andrew Morton, Dongyang Wang Cc: christian.brauner, ebiederm, legion, varad.gautam, linux-kernel, Davidlohr Bueso Hi Dongyang, On 1/26/22 03:42, Andrew Morton wrote: > On Tue, 18 Jan 2022 17:19:52 +0800 Dongyang Wang <dongyang.wang@windriver.com> wrote: > >> [786058.308965] Unable to handle kernel paging request at virtual address 01000004 >> [786058.316286] pgd = 38a99693 >> [786058.319080] [01000004] *pgd=07800003, *pmd=00000000 >> [786058.324056] Internal error: Oops: 206 [#1] PREEMPT SMP ARM >> [786058.324100] CPU: PID: Comm: Tainted: G C >> [786058.324102] Hardware name: >> [786058.324114] PC is at __copy_to_user_std+0x4c/0x3c4 >> [786058.324120] LR is at store_msg+0xc0/0xe8 >> [786058.324124] pc : [<c0c0587c>] lr : [<c0871d04>] psr: 20010013 >> [786058.324126] sp : c3503ec4 ip : 00000000 fp : b4c9a660 >> [786058.324129] r10: c4228dc0 r9 : c3502000 r8 : 00000ffc >> [786058.324132] r7 : 01000000 r6 : 546d3f8b r5 : b4911690 r4 : 00000ffc >> [786058.324134] r3 : 00000000 r2 : 00000f7c r1 : 01000004 r0 : b4911690 >> [786058.324139] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user >> [786058.324142] Control: 30c5387d Table: 0edc2040 DAC: 55555555 >> [786058.324145] Process (pid: , stack limit = 0x25018bdf) Why is process and pid: empty? Is this some kind of kernel process calling? >> [786058.324148] Stack: (0xc3503ec4 to 0xc3504000) >> [786058.324153] 3ec0: b4911690 546d3f8b 01000000 00000ffc b4911690 00000ffc 00000000 >> [786058.324157] 3ee0: 00000ffc c0871d04 546d4f73 c3407801 c3503f28 c3407800 00000000 b49106a8 >> [786058.324161] 3f00: c4228dc0 c087abd4 00000002 b49106a8 617b9d03 00000000 00000000 c121d508 >> [786058.324165] 3f20: 00000000 bf06a1a8 d1b634cc 16b26e77 c5af5280 00000100 00000200 db806540 >> [786058.324170] 3f40: 00000001 c121d508 00000008 0000005c 00000000 00010008 b49106a8 c0601208 >> [786058.324173] 3f60: c3502000 00000040 b4c9a660 c087b474 c3503f78 c121d508 617b9d03 00000000 >> [786058.324177] 3f80: 2303d6cc 00000115 c0601208 c121d508 b4c9a660 b4c9a660 00000001 b49106a8 >> [786058.324181] 3fa0: 00000115 c06011dc b4c9a660 00000001 0000005c b49106a8 00010008 00000000 >> [786058.324185] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 >> [786058.324189] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 800d0030 0000005c 00000000 00000000 >> [786058.324201] [<c0c0587c>] (__copy_to_user_std) from [<c0871d04>] (store_msg+0xc0/0xe8) >> [786058.324211] [<c0871d04>] (store_msg) from [<c087abd4>] (do_mq_timedreceive+0x29c/0x484) >> [786058.324218] [<c087abd4>] (do_mq_timedreceive) from [<c087b474>] (sys_mq_timedreceive+0x88/0xbc) >> [786058.324226] [<c087b474>] (sys_mq_timedreceive) from [<c06011dc>] (__sys_trace_return+0x0/0x10) >> [786058.324229] Exception stack(0xc3503fa8 to 0xc3503ff0) >> [786058.324233] 3fa0: b4c9a660 00000001 0000005c b49106a8 00010008 00000000 >> [786058.324236] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 >> [786058.324239] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 >> [786058.324247] Code: ba000002 f5d1f03c f5d1f05c f5d1f07c (e8b151f8) >> [786058.601808] ---[ end trace 0000000000000002 ]--- >> >> ... >> > Please describe the circumstances under which this occurs. > > Please also describe how your proposed patch fixes this. > >> --- a/ipc/mqueue.c >> +++ b/ipc/mqueue.c >> @@ -1211,6 +1211,12 @@ static int do_mq_timedreceive(mqd_t mqdes, char __user *u_msg_ptr, >> goto out_fput; >> } >> >> + /* checks if buffer is invalid */ >> + if ((unsigned long) u_msg_ptr < PAGE_SIZE) { >> + ret = -EINVAL; >> + goto out_fput; >> + } >> + I do not think that this will solve the problem: u_msg_ptr is user controlled. If < PAGE_SIZE causes issues, then 2^32-1, or PAGE_SIZE+1 will cause issues as well. What is the kernel version where you have observed the issue? >> /* >> * msg_insert really wants us to have a valid, spare node struct so >> * it doesn't have to kmalloc a GFP_ATOMIC allocation, but it will >> @@ -1262,6 +1268,10 @@ static int do_mq_timedreceive(mqd_t mqdes, char __user *u_msg_ptr, >> store_msg(u_msg_ptr, msg_ptr, msg_ptr->m_ts)) { >> ret = -EFAULT; >> } >> + >> + if (msg_len < ret) >> + ret = -EMSGSIZE; >> + Why this change? EMSGSIZE means right now: wrong parameters, no message removed from the queue. But: in the line you modify, a message was already removed from the queue. Thus I would not consider EMSGSIZE as appropriate. >> free_msg(msg_ptr); >> } >> out_fput: >> -- >> 2.26.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] To fix the below failure of handling page fault caused by the invalid input from user. 2022-01-26 16:52 ` Manfred Spraul @ 2022-01-29 15:02 ` Dongyang Wang 2022-01-29 15:59 ` Manfred Spraul 0 siblings, 1 reply; 7+ messages in thread From: Dongyang Wang @ 2022-01-29 15:02 UTC (permalink / raw) To: manfred Cc: akpm, christian.brauner, dave, dongyang.wang, ebiederm, legion, linux-kernel, varad.gautam Hi Manfred and Andrew, Thanks for your feedback. >Hi Dongyang, > >On 1/26/22 03:42, Andrew Morton wrote: >> On Tue, 18 Jan 2022 17:19:52 +0800 Dongyang Wang <dongyang.wang@windriver.com> wrote: >> >>> [786058.308965] Unable to handle kernel paging request at virtual address 01000004 >>> [786058.316286] pgd = 38a99693 >>> [786058.319080] [01000004] *pgd=07800003, *pmd=00000000 >>> [786058.324056] Internal error: Oops: 206 [#1] PREEMPT SMP ARM >>> [786058.324100] CPU: PID: Comm: Tainted: G C >>> [786058.324102] Hardware name: >>> [786058.324114] PC is at __copy_to_user_std+0x4c/0x3c4 >>> [786058.324120] LR is at store_msg+0xc0/0xe8 >>> [786058.324124] pc : [<c0c0587c>] lr : [<c0871d04>] psr: 20010013 >>> [786058.324126] sp : c3503ec4 ip : 00000000 fp : b4c9a660 >>> [786058.324129] r10: c4228dc0 r9 : c3502000 r8 : 00000ffc >>> [786058.324132] r7 : 01000000 r6 : 546d3f8b r5 : b4911690 r4 : 00000ffc >>> [786058.324134] r3 : 00000000 r2 : 00000f7c r1 : 01000004 r0 : b4911690 >>> [786058.324139] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user >>> [786058.324142] Control: 30c5387d Table: 0edc2040 DAC: 55555555 >>> [786058.324145] Process (pid: , stack limit = 0x25018bdf) > >Why is process and pid: empty? Is this some kind of kernel process calling? The pid is 8369, it's a userspace app. > >>> [786058.324148] Stack: (0xc3503ec4 to 0xc3504000) >>> [786058.324153] 3ec0: b4911690 546d3f8b 01000000 00000ffc b4911690 00000ffc 00000000 >>> [786058.324157] 3ee0: 00000ffc c0871d04 546d4f73 c3407801 c3503f28 c3407800 00000000 b49106a8 >>> [786058.324161] 3f00: c4228dc0 c087abd4 00000002 b49106a8 617b9d03 00000000 00000000 c121d508 >>> [786058.324165] 3f20: 00000000 bf06a1a8 d1b634cc 16b26e77 c5af5280 00000100 00000200 db806540 >>> [786058.324170] 3f40: 00000001 c121d508 00000008 0000005c 00000000 00010008 b49106a8 c0601208 >>> [786058.324173] 3f60: c3502000 00000040 b4c9a660 c087b474 c3503f78 c121d508 617b9d03 00000000 >>> [786058.324177] 3f80: 2303d6cc 00000115 c0601208 c121d508 b4c9a660 b4c9a660 00000001 b49106a8 >>> [786058.324181] 3fa0: 00000115 c06011dc b4c9a660 00000001 0000005c b49106a8 00010008 00000000 >>> [786058.324185] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 >>> [786058.324189] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 800d0030 0000005c 00000000 00000000 >>> [786058.324201] [<c0c0587c>] (__copy_to_user_std) from [<c0871d04>] (store_msg+0xc0/0xe8) >>> [786058.324211] [<c0871d04>] (store_msg) from [<c087abd4>] (do_mq_timedreceive+0x29c/0x484) >>> [786058.324218] [<c087abd4>] (do_mq_timedreceive) from [<c087b474>] (sys_mq_timedreceive+0x88/0xbc) >>> [786058.324226] [<c087b474>] (sys_mq_timedreceive) from [<c06011dc>] (__sys_trace_return+0x0/0x10) >>> [786058.324229] Exception stack(0xc3503fa8 to 0xc3503ff0) >>> [786058.324233] 3fa0: b4c9a660 00000001 0000005c b49106a8 00010008 00000000 >>> [786058.324236] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 >>> [786058.324239] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 >>> [786058.324247] Code: ba000002 f5d1f03c f5d1f05c f5d1f07c (e8b151f8) >>> [786058.601808] ---[ end trace 0000000000000002 ]--- >>> >>> ... >>> >> Please describe the circumstances under which this occurs. The system crashed after running 9 days. Unfortunately, we don't know how the user space APP running, it's a black box for us. >> >> Please also describe how your proposed patch fixes this. Do some check for the input parameters from the userspace. This crash may be caused by the APP, and I know the right way is to change the APP's behavior. But I think maybe the kernel side also can do some checks for the input parameters? To improve the robustness? I'm not sure. but I want to have a try, so I pushed this patch. Thanks :) >> >>> --- a/ipc/mqueue.c >>> +++ b/ipc/mqueue.c >>> @@ -1211,6 +1211,12 @@ static int do_mq_timedreceive(mqd_t mqdes, char __user *u_msg_ptr, >>> goto out_fput; >>> } >>> >>> + /* checks if buffer is invalid */ >>> + if ((unsigned long) u_msg_ptr < PAGE_SIZE) { >>> + ret = -EINVAL; >>> + goto out_fput; >>> + } >>> + > >I do not think that this will solve the problem: u_msg_ptr is user >controlled. > >If < PAGE_SIZE causes issues, then 2^32-1, or PAGE_SIZE+1 will cause >issues as well. Thank you! you are right. I want to check the validity of the u_msg_ptr. But it seems this check is not enough now. > >What is the kernel version where you have observed the issue? > The kernel version is 4.18. >>> /* >>> * msg_insert really wants us to have a valid, spare node struct so >>> * it doesn't have to kmalloc a GFP_ATOMIC allocation, but it will >>> @@ -1262,6 +1268,10 @@ static int do_mq_timedreceive(mqd_t mqdes, char __user *u_msg_ptr, >>> store_msg(u_msg_ptr, msg_ptr, msg_ptr->m_ts)) { >>> ret = -EFAULT; >>> } >>> + >>> + if (msg_len < ret) >>> + ret = -EMSGSIZE; >>> + > >Why this change? > >EMSGSIZE means right now: wrong parameters, no message removed from the >queue. > >But: in the line you modify, a message was already removed from the >queue. Thus I would not consider EMSGSIZE as appropriate. > >>> free_msg(msg_ptr); >>> } >>> out_fput: >>> -- >>> 2.26.1 Sorry, rewrite this part like this: if (ret == 0) { ret = msg_ptr->m_ts; - if ((u_msg_prio && put_user(msg_ptr->m_type, u_msg_prio)) || - store_msg(u_msg_ptr, msg_ptr, msg_ptr->m_ts)) { + if (u_msg_prio && put_user(msg_ptr->m_type, u_msg_prio)) ret = -EFAULT; - } + if (msg_len < ret) + ret = -EMSGSIZE; + if (store_msg(u_msg_ptr, msg_ptr, msg_ptr->m_ts)) + ret = -EFAULT; + free_msg(msg_ptr); } Add a check before call the store_msg. About the return value: EMSGSIZE. How about this meaning? #define EMSGSIZE 90 /* Message too long */ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] To fix the below failure of handling page fault caused by the invalid input from user. 2022-01-29 15:02 ` Dongyang Wang @ 2022-01-29 15:59 ` Manfred Spraul 2022-02-09 7:28 ` Dongyang Wang 0 siblings, 1 reply; 7+ messages in thread From: Manfred Spraul @ 2022-01-29 15:59 UTC (permalink / raw) To: Dongyang Wang Cc: akpm, christian.brauner, dave, ebiederm, legion, linux-kernel, varad.gautam Hi Dongyang, On 1/29/22 16:02, Dongyang Wang wrote: > Hi Manfred and Andrew, > > Thanks for your feedback. > >> Hi Dongyang, >> >> On 1/26/22 03:42, Andrew Morton wrote: >>> On Tue, 18 Jan 2022 17:19:52 +0800 Dongyang Wang <dongyang.wang@windriver.com> wrote: >>> >>>> [786058.308965] Unable to handle kernel paging request at virtual address 01000004 >>>> [786058.316286] pgd = 38a99693 >>>> [786058.319080] [01000004] *pgd=07800003, *pmd=00000000 >>>> [786058.324056] Internal error: Oops: 206 [#1] PREEMPT SMP ARM >>>> [786058.324100] CPU: PID: Comm: Tainted: G C >>>> [786058.324102] Hardware name: >>>> [786058.324114] PC is at __copy_to_user_std+0x4c/0x3c4 >>>> [786058.324120] LR is at store_msg+0xc0/0xe8 >>>> [786058.324124] pc : [<c0c0587c>] lr : [<c0871d04>] psr: 20010013 >>>> [786058.324126] sp : c3503ec4 ip : 00000000 fp : b4c9a660 >>>> [786058.324129] r10: c4228dc0 r9 : c3502000 r8 : 00000ffc >>>> [786058.324132] r7 : 01000000 r6 : 546d3f8b r5 : b4911690 r4 : 00000ffc >>>> [786058.324134] r3 : 00000000 r2 : 00000f7c r1 : 01000004 r0 : b4911690 >>>> [786058.324139] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user >>>> [786058.324142] Control: 30c5387d Table: 0edc2040 DAC: 55555555 >>>> [786058.324145] Process (pid: , stack limit = 0x25018bdf) >> Why is process and pid: empty? Is this some kind of kernel process calling? > The pid is 8369, it's a userspace app. > >>>> [786058.324148] Stack: (0xc3503ec4 to 0xc3504000) >>>> [786058.324153] 3ec0: b4911690 546d3f8b 01000000 00000ffc b4911690 00000ffc 00000000 >>>> [786058.324157] 3ee0: 00000ffc c0871d04 546d4f73 c3407801 c3503f28 c3407800 00000000 b49106a8 >>>> [786058.324161] 3f00: c4228dc0 c087abd4 00000002 b49106a8 617b9d03 00000000 00000000 c121d508 >>>> [786058.324165] 3f20: 00000000 bf06a1a8 d1b634cc 16b26e77 c5af5280 00000100 00000200 db806540 >>>> [786058.324170] 3f40: 00000001 c121d508 00000008 0000005c 00000000 00010008 b49106a8 c0601208 >>>> [786058.324173] 3f60: c3502000 00000040 b4c9a660 c087b474 c3503f78 c121d508 617b9d03 00000000 >>>> [786058.324177] 3f80: 2303d6cc 00000115 c0601208 c121d508 b4c9a660 b4c9a660 00000001 b49106a8 >>>> [786058.324181] 3fa0: 00000115 c06011dc b4c9a660 00000001 0000005c b49106a8 00010008 00000000 >>>> [786058.324185] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 >>>> [786058.324189] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 800d0030 0000005c 00000000 00000000 >>>> [786058.324201] [<c0c0587c>] (__copy_to_user_std) from [<c0871d04>] (store_msg+0xc0/0xe8) I would search here: __copy_to_user_std should fail if the address is invalid. For whatever reasons, it produces a page fault. First: Is this reproducible? Does it fail immediately if you pass an invalid value to mq_timedreceive()? https://elixir.bootlin.com/linux/v4.18.20/source/arch/arm/include/asm/uaccess.h#L464 It seems ARM has special optimizations (CONFIG_UACCESS_WITH_MEMCPY), and I cannot see if this is MMU or NO_MMU > The kernel version is 4.18. Ok. -- Manfred ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] To fix the below failure of handling page fault caused by the invalid input from user. 2022-01-29 15:59 ` Manfred Spraul @ 2022-02-09 7:28 ` Dongyang Wang 2022-03-04 7:33 ` Dongyang Wang 0 siblings, 1 reply; 7+ messages in thread From: Dongyang Wang @ 2022-02-09 7:28 UTC (permalink / raw) To: manfred Cc: akpm, christian.brauner, dave, dongyang.wang, ebiederm, legion, linux-kernel, varad.gautam Hi Manfred, I'm sorry for the late reply (we have a Chinese Lunar New Year). >>> On 1/26/22 03:42, Andrew Morton wrote: >>>> On Tue, 18 Jan 2022 17:19:52 +0800 Dongyang Wang <dongyang.wang@windriver.com> wrote: >>>> >>>>> [786058.308965] Unable to handle kernel paging request at virtual address 01000004 >>>>> [786058.316286] pgd = 38a99693 >>>>> [786058.319080] [01000004] *pgd=07800003, *pmd=00000000 >>>>> [786058.324056] Internal error: Oops: 206 [#1] PREEMPT SMP ARM >>>>> [786058.324100] CPU: PID: Comm: Tainted: G C >>>>> [786058.324102] Hardware name: >>>>> [786058.324114] PC is at __copy_to_user_std+0x4c/0x3c4 >>>>> [786058.324120] LR is at store_msg+0xc0/0xe8 >>>>> [786058.324124] pc : [<c0c0587c>] lr : [<c0871d04>] psr: 20010013 >>>>> [786058.324126] sp : c3503ec4 ip : 00000000 fp : b4c9a660 >>>>> [786058.324129] r10: c4228dc0 r9 : c3502000 r8 : 00000ffc >>>>> [786058.324132] r7 : 01000000 r6 : 546d3f8b r5 : b4911690 r4 : 00000ffc >>>>> [786058.324134] r3 : 00000000 r2 : 00000f7c r1 : 01000004 r0 : b4911690 >>>>> [786058.324139] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user >>>>> [786058.324142] Control: 30c5387d Table: 0edc2040 DAC: 55555555 >>>>> [786058.324145] Process (pid: , stack limit = 0x25018bdf) >>> Why is process and pid: empty? Is this some kind of kernel process calling? >> The pid is 8369, it's a userspace app. >> >>>>> [786058.324148] Stack: (0xc3503ec4 to 0xc3504000) >>>>> [786058.324153] 3ec0: b4911690 546d3f8b 01000000 00000ffc b4911690 00000ffc 00000000 >>>>> [786058.324157] 3ee0: 00000ffc c0871d04 546d4f73 c3407801 c3503f28 c3407800 00000000 b49106a8 >>>>> [786058.324161] 3f00: c4228dc0 c087abd4 00000002 b49106a8 617b9d03 00000000 00000000 c121d508 >>>>> [786058.324165] 3f20: 00000000 bf06a1a8 d1b634cc 16b26e77 c5af5280 00000100 00000200 db806540 >>>>> [786058.324170] 3f40: 00000001 c121d508 00000008 0000005c 00000000 00010008 b49106a8 c0601208 >>>>> [786058.324173] 3f60: c3502000 00000040 b4c9a660 c087b474 c3503f78 c121d508 617b9d03 00000000 >>>>> [786058.324177] 3f80: 2303d6cc 00000115 c0601208 c121d508 b4c9a660 b4c9a660 00000001 b49106a8 >>>>> [786058.324181] 3fa0: 00000115 c06011dc b4c9a660 00000001 0000005c b49106a8 00010008 00000000 >>>>> [786058.324185] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 >>>>> [786058.324189] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 800d0030 0000005c 00000000 00000000 >>>>> [786058.324201] [<c0c0587c>] (__copy_to_user_std) from [<c0871d04>] (store_msg+0xc0/0xe8) > >I would search here: __copy_to_user_std should fail if the address is >invalid. > >For whatever reasons, it produces a page fault. Totally agree with you. >First: Is this reproducible? Does it fail immediately if you pass an >invalid value to mq_timedreceive()? This crash info is from another team. It can't be reproduced until now. Yesterday, I changed the 'u_msg_ptr' and 'msg_ptr->m_ts' to a wrong value, but don't cause crash. I will watch this part. >https://elixir.bootlin.com/linux/v4.18.20/source/arch/arm/include/asm/uaccess.h#L464 > >It seems ARM has special optimizations (CONFIG_UACCESS_WITH_MEMCPY), and >I cannot see if this is MMU or NO_MMU This is MMU. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] To fix the below failure of handling page fault caused by the invalid input from user. 2022-02-09 7:28 ` Dongyang Wang @ 2022-03-04 7:33 ` Dongyang Wang 0 siblings, 0 replies; 7+ messages in thread From: Dongyang Wang @ 2022-03-04 7:33 UTC (permalink / raw) To: dongyang.wang Cc: akpm, christian.brauner, dave, ebiederm, legion, linux-kernel, manfred, varad.gautam >>>> On 1/26/22 03:42, Andrew Morton wrote: >>>>> On Tue, 18 Jan 2022 17:19:52 +0800 Dongyang Wang <dongyang.wang@windriver.com> wrote: >>>>> >>>>>> [786058.308965] Unable to handle kernel paging request at virtual address 01000004 >>>>>> [786058.316286] pgd = 38a99693 >>>>>> [786058.319080] [01000004] *pgd=07800003, *pmd=00000000 >>>>>> [786058.324056] Internal error: Oops: 206 [#1] PREEMPT SMP ARM >>>>>> [786058.324100] CPU: PID: Comm: Tainted: G C >>>>>> [786058.324102] Hardware name: >>>>>> [786058.324114] PC is at __copy_to_user_std+0x4c/0x3c4 >>>>>> [786058.324120] LR is at store_msg+0xc0/0xe8 >>>>>> [786058.324124] pc : [<c0c0587c>] lr : [<c0871d04>] psr: 20010013 >>>>>> [786058.324126] sp : c3503ec4 ip : 00000000 fp : b4c9a660 >>>>>> [786058.324129] r10: c4228dc0 r9 : c3502000 r8 : 00000ffc >>>>>> [786058.324132] r7 : 01000000 r6 : 546d3f8b r5 : b4911690 r4 : 00000ffc >>>>>> [786058.324134] r3 : 00000000 r2 : 00000f7c r1 : 01000004 r0 : b4911690 >>>>>> [786058.324139] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user >>>>>> [786058.324142] Control: 30c5387d Table: 0edc2040 DAC: 55555555 >>>>>> [786058.324145] Process (pid: , stack limit = 0x25018bdf) >>>> Why is process and pid: empty? Is this some kind of kernel process calling? >>> The pid is 8369, it's a userspace app. >>> >>>>>> [786058.324148] Stack: (0xc3503ec4 to 0xc3504000) >>>>>> [786058.324153] 3ec0: b4911690 546d3f8b 01000000 00000ffc b4911690 00000ffc 00000000 >>>>>> [786058.324157] 3ee0: 00000ffc c0871d04 546d4f73 c3407801 c3503f28 c3407800 00000000 b49106a8 >>>>>> [786058.324161] 3f00: c4228dc0 c087abd4 00000002 b49106a8 617b9d03 00000000 00000000 c121d508 >>>>>> [786058.324165] 3f20: 00000000 bf06a1a8 d1b634cc 16b26e77 c5af5280 00000100 00000200 db806540 >>>>>> [786058.324170] 3f40: 00000001 c121d508 00000008 0000005c 00000000 00010008 b49106a8 c0601208 >>>>>> [786058.324173] 3f60: c3502000 00000040 b4c9a660 c087b474 c3503f78 c121d508 617b9d03 00000000 >>>>>> [786058.324177] 3f80: 2303d6cc 00000115 c0601208 c121d508 b4c9a660 b4c9a660 00000001 b49106a8 >>>>>> [786058.324181] 3fa0: 00000115 c06011dc b4c9a660 00000001 0000005c b49106a8 00010008 00000000 >>>>>> [786058.324185] 3fc0: b4c9a660 00000001 b49106a8 00000115 00000000 b4c9b400 00000000 b4c9a660 >>>>>> [786058.324189] 3fe0: 00000115 b4c9a650 b6b253bd b6b254b6 800d0030 0000005c 00000000 00000000 >>>>>> [786058.324201] [<c0c0587c>] (__copy_to_user_std) from [<c0871d04>] (store_msg+0xc0/0xe8) >> >>I would search here: __copy_to_user_std should fail if the address is >>invalid. >> >>For whatever reasons, it produces a page fault. > >Totally agree with you. > >>First: Is this reproducible? Does it fail immediately if you pass an >>invalid value to mq_timedreceive()? > >This crash info is from another team. It can't be reproduced until now. >Yesterday, I changed the 'u_msg_ptr' and 'msg_ptr->m_ts' to a wrong value, but don't cause crash. >I will watch this part. Status update: Sorry, after changing the "u_msg_ptr", "msg_ptr->m_ts" to the wrong value, I still can't reproduce this issue. >>https://elixir.bootlin.com/linux/v4.18.20/source/arch/arm/include/asm/uaccess.h#L464 >> >>It seems ARM has special optimizations (CONFIG_UACCESS_WITH_MEMCPY), and >>I cannot see if this is MMU or NO_MMU > >This is MMU. Best Regards, Dongyang ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-04 7:34 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-18 9:19 [PATCH] To fix the below failure of handling page fault caused by the invalid input from user Dongyang Wang 2022-01-26 2:42 ` Andrew Morton 2022-01-26 16:52 ` Manfred Spraul 2022-01-29 15:02 ` Dongyang Wang 2022-01-29 15:59 ` Manfred Spraul 2022-02-09 7:28 ` Dongyang Wang 2022-03-04 7:33 ` Dongyang Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox