2018-09-17 03:27:50

by He Zhe

Subject: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

From: He Zhe <[email protected]>

setup_io_tlb_npages does not check input argument before passing it
to isdigit. The argument would be a NULL pointer if "swiotlb", without
its value, is set in command line and thus causes the following panic.

PANIC: early exception 0xe3 IP 10:ffffffffbb9b8e9f error 0 cr2 0x0
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc3-yocto-standard+ #9
[ 0.000000] RIP: 0010:setup_io_tlb_npages+0xf/0x95
...
[ 0.000000] Call Trace:
[ 0.000000] do_early_param+0x57/0x8e
[ 0.000000] parse_args+0x208/0x320
[ 0.000000] ? rdinit_setup+0x30/0x30
[ 0.000000] parse_early_options+0x29/0x2d
[ 0.000000] ? rdinit_setup+0x30/0x30
[ 0.000000] parse_early_param+0x36/0x4d
[ 0.000000] setup_arch+0x336/0x99e
[ 0.000000] start_kernel+0x6f/0x4e6
[ 0.000000] x86_64_start_reservations+0x24/0x26
[ 0.000000] x86_64_start_kernel+0x6f/0x72
[ 0.000000] secondary_startup_64+0xa4/0xb0

This patch adds a check to prevent the panic.

Signed-off-by: He Zhe <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
kernel/dma/swiotlb.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 4f8a6db..46fc34e 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -109,6 +109,11 @@ static int late_alloc;
static int __init
setup_io_tlb_npages(char *str)
{
+ if (!str) {
+ pr_err("Config string not provided\n");
+ return -EINVAL;
+ }
+
if (isdigit(*str)) {
io_tlb_nslabs = simple_strtoul(str, &str, 0);
/* avoid tail segment of size < IO_TLB_SEGSIZE */
--
2.7.4

2018-09-22 12:57:43

by He Zhe

Subject: Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

May I have your input?

Thanks,
Zhe

On 2018年09月17日 11:27, [email protected] wrote:
> From: He Zhe <[email protected]>
>
> setup_io_tlb_npages does not check input argument before passing it
> to isdigit. The argument would be a NULL pointer if "swiotlb", without
> its value, is set in command line and thus causes the following panic.
>
> PANIC: early exception 0xe3 IP 10:ffffffffbb9b8e9f error 0 cr2 0x0
> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc3-yocto-standard+ #9
> [ 0.000000] RIP: 0010:setup_io_tlb_npages+0xf/0x95
> ...
> [ 0.000000] Call Trace:
> [ 0.000000] do_early_param+0x57/0x8e
> [ 0.000000] parse_args+0x208/0x320
> [ 0.000000] ? rdinit_setup+0x30/0x30
> [ 0.000000] parse_early_options+0x29/0x2d
> [ 0.000000] ? rdinit_setup+0x30/0x30
> [ 0.000000] parse_early_param+0x36/0x4d
> [ 0.000000] setup_arch+0x336/0x99e
> [ 0.000000] start_kernel+0x6f/0x4e6
> [ 0.000000] x86_64_start_reservations+0x24/0x26
> [ 0.000000] x86_64_start_kernel+0x6f/0x72
> [ 0.000000] secondary_startup_64+0xa4/0xb0
>
> This patch adds a check to prevent the panic.
>
> Signed-off-by: He Zhe <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> kernel/dma/swiotlb.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 4f8a6db..46fc34e 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -109,6 +109,11 @@ static int late_alloc;
> static int __init
> setup_io_tlb_npages(char *str)
> {
> + if (!str) {
> + pr_err("Config string not provided\n");
> + return -EINVAL;
> + }
> +
> if (isdigit(*str)) {
> io_tlb_nslabs = simple_strtoul(str, &str, 0);
> /* avoid tail segment of size < IO_TLB_SEGSIZE */

2018-10-22 20:11:34

by Konrad Rzeszutek Wilk

Subject: Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

On Sat, Sep 22, 2018 at 08:56:58PM +0800, He Zhe wrote:
> May I have your input?

Alternatively would it make more sense for it to assume some default
value?
>
> Thanks,
> Zhe
>
> On 2018年09月17日 11:27, [email protected] wrote:
> > From: He Zhe <[email protected]>
> >
> > setup_io_tlb_npages does not check input argument before passing it
> > to isdigit. The argument would be a NULL pointer if "swiotlb", without
> > its value, is set in command line and thus causes the following panic.
> >
> > PANIC: early exception 0xe3 IP 10:ffffffffbb9b8e9f error 0 cr2 0x0
> > [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc3-yocto-standard+ #9
> > [ 0.000000] RIP: 0010:setup_io_tlb_npages+0xf/0x95
> > ...
> > [ 0.000000] Call Trace:
> > [ 0.000000] do_early_param+0x57/0x8e
> > [ 0.000000] parse_args+0x208/0x320
> > [ 0.000000] ? rdinit_setup+0x30/0x30
> > [ 0.000000] parse_early_options+0x29/0x2d
> > [ 0.000000] ? rdinit_setup+0x30/0x30
> > [ 0.000000] parse_early_param+0x36/0x4d
> > [ 0.000000] setup_arch+0x336/0x99e
> > [ 0.000000] start_kernel+0x6f/0x4e6
> > [ 0.000000] x86_64_start_reservations+0x24/0x26
> > [ 0.000000] x86_64_start_kernel+0x6f/0x72
> > [ 0.000000] secondary_startup_64+0xa4/0xb0
> >
> > This patch adds a check to prevent the panic.
> >
> > Signed-off-by: He Zhe <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > kernel/dma/swiotlb.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > index 4f8a6db..46fc34e 100644
> > --- a/kernel/dma/swiotlb.c
> > +++ b/kernel/dma/swiotlb.c
> > @@ -109,6 +109,11 @@ static int late_alloc;
> > static int __init
> > setup_io_tlb_npages(char *str)
> > {
> > + if (!str) {
> > + pr_err("Config string not provided\n");
> > + return -EINVAL;
> > + }
> > +
> > if (isdigit(*str)) {
> > io_tlb_nslabs = simple_strtoul(str, &str, 0);
> > /* avoid tail segment of size < IO_TLB_SEGSIZE */
>
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

2018-10-23 11:17:08

by He Zhe

Subject: Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

On 2018/10/23 03:29, Konrad Rzeszutek Wilk wrote:
> On Sat, Sep 22, 2018 at 08:56:58PM +0800, He Zhe wrote:
>> May I have your input?
> Alternatively would it make more sense for it to assume some default
> value?

Maybe, but the original code has no default value and I have no idea
what default value is proper here.

Zhe

>> Thanks,
>> Zhe
>>
>> On 2018年09月17日 11:27, [email protected] wrote:
>>> From: He Zhe <[email protected]>
>>>
>>> setup_io_tlb_npages does not check input argument before passing it
>>> to isdigit. The argument would be a NULL pointer if "swiotlb", without
>>> its value, is set in command line and thus causes the following panic.
>>>
>>> PANIC: early exception 0xe3 IP 10:ffffffffbb9b8e9f error 0 cr2 0x0
>>> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc3-yocto-standard+ #9
>>> [ 0.000000] RIP: 0010:setup_io_tlb_npages+0xf/0x95
>>> ...
>>> [ 0.000000] Call Trace:
>>> [ 0.000000] do_early_param+0x57/0x8e
>>> [ 0.000000] parse_args+0x208/0x320
>>> [ 0.000000] ? rdinit_setup+0x30/0x30
>>> [ 0.000000] parse_early_options+0x29/0x2d
>>> [ 0.000000] ? rdinit_setup+0x30/0x30
>>> [ 0.000000] parse_early_param+0x36/0x4d
>>> [ 0.000000] setup_arch+0x336/0x99e
>>> [ 0.000000] start_kernel+0x6f/0x4e6
>>> [ 0.000000] x86_64_start_reservations+0x24/0x26
>>> [ 0.000000] x86_64_start_kernel+0x6f/0x72
>>> [ 0.000000] secondary_startup_64+0xa4/0xb0
>>>
>>> This patch adds a check to prevent the panic.
>>>
>>> Signed-off-by: He Zhe <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> ---
>>> kernel/dma/swiotlb.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>>> index 4f8a6db..46fc34e 100644
>>> --- a/kernel/dma/swiotlb.c
>>> +++ b/kernel/dma/swiotlb.c
>>> @@ -109,6 +109,11 @@ static int late_alloc;
>>> static int __init
>>> setup_io_tlb_npages(char *str)
>>> {
>>> + if (!str) {
>>> + pr_err("Config string not provided\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> if (isdigit(*str)) {
>>> io_tlb_nslabs = simple_strtoul(str, &str, 0);
>>> /* avoid tail segment of size < IO_TLB_SEGSIZE */
>> _______________________________________________
>> iommu mailing list
>> [email protected]
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu

2018-10-30 11:09:20

by He Zhe

Subject: Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

On 2018/10/23 19:14, He Zhe wrote:
>
> On 2018/10/23 03:29, Konrad Rzeszutek Wilk wrote:
>> On Sat, Sep 22, 2018 at 08:56:58PM +0800, He Zhe wrote:
>>> May I have your input?
>> Alternatively would it make more sense for it to assume some default
>> value?
> Maybe, but the original code has no default value and I have no idea
> what default value is proper here.

Can anyone give some suggestions? Though I'd prefer to do nothing when
no option is provided.

Thanks,
Zhe

>
> Zhe
>
>>> Thanks,
>>> Zhe
>>>
>>> On 2018年09月17日 11:27, [email protected] wrote:
>>>> From: He Zhe <[email protected]>
>>>>
>>>> setup_io_tlb_npages does not check input argument before passing it
>>>> to isdigit. The argument would be a NULL pointer if "swiotlb", without
>>>> its value, is set in command line and thus causes the following panic.
>>>>
>>>> PANIC: early exception 0xe3 IP 10:ffffffffbb9b8e9f error 0 cr2 0x0
>>>> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc3-yocto-standard+ #9
>>>> [ 0.000000] RIP: 0010:setup_io_tlb_npages+0xf/0x95
>>>> ...
>>>> [ 0.000000] Call Trace:
>>>> [ 0.000000] do_early_param+0x57/0x8e
>>>> [ 0.000000] parse_args+0x208/0x320
>>>> [ 0.000000] ? rdinit_setup+0x30/0x30
>>>> [ 0.000000] parse_early_options+0x29/0x2d
>>>> [ 0.000000] ? rdinit_setup+0x30/0x30
>>>> [ 0.000000] parse_early_param+0x36/0x4d
>>>> [ 0.000000] setup_arch+0x336/0x99e
>>>> [ 0.000000] start_kernel+0x6f/0x4e6
>>>> [ 0.000000] x86_64_start_reservations+0x24/0x26
>>>> [ 0.000000] x86_64_start_kernel+0x6f/0x72
>>>> [ 0.000000] secondary_startup_64+0xa4/0xb0
>>>>
>>>> This patch adds a check to prevent the panic.
>>>>
>>>> Signed-off-by: He Zhe <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> ---
>>>> kernel/dma/swiotlb.c | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>>>> index 4f8a6db..46fc34e 100644
>>>> --- a/kernel/dma/swiotlb.c
>>>> +++ b/kernel/dma/swiotlb.c
>>>> @@ -109,6 +109,11 @@ static int late_alloc;
>>>> static int __init
>>>> setup_io_tlb_npages(char *str)
>>>> {
>>>> + if (!str) {
>>>> + pr_err("Config string not provided\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> if (isdigit(*str)) {
>>>> io_tlb_nslabs = simple_strtoul(str, &str, 0);
>>>> /* avoid tail segment of size < IO_TLB_SEGSIZE */
>>> _______________________________________________
>>> iommu mailing list
>>> [email protected]
>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

2018-12-02 12:31:28

by Konrad Rzeszutek Wilk

Subject: Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

On Tue, Oct 30, 2018 at 07:06:10PM +0800, He Zhe wrote:
>
>
> On 2018/10/23 19:14, He Zhe wrote:
> >
> > On 2018/10/23 03:29, Konrad Rzeszutek Wilk wrote:
> >> On Sat, Sep 22, 2018 at 08:56:58PM +0800, He Zhe wrote:
> >>> May I have your input?
> >> Alternatively would it make more sense for it to assume some default
> >> value?
> > Maybe, but the original code has no default value and I have no idea
> > what default value is proper here.
>
> Can anyone give some suggestions? Though I'd prefer to do nothing when
> no option is provided.

One provided the paramter for a reason. I would just do a small one, say
4MB.
>
> Thanks,
> Zhe
>
> >
> > Zhe
> >
> >>> Thanks,
> >>> Zhe
> >>>
> >>> On 2018年09月17日 11:27, [email protected] wrote:
> >>>> From: He Zhe <[email protected]>
> >>>>
> >>>> setup_io_tlb_npages does not check input argument before passing it
> >>>> to isdigit. The argument would be a NULL pointer if "swiotlb", without
> >>>> its value, is set in command line and thus causes the following panic.
> >>>>
> >>>> PANIC: early exception 0xe3 IP 10:ffffffffbb9b8e9f error 0 cr2 0x0
> >>>> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc3-yocto-standard+ #9
> >>>> [ 0.000000] RIP: 0010:setup_io_tlb_npages+0xf/0x95
> >>>> ...
> >>>> [ 0.000000] Call Trace:
> >>>> [ 0.000000] do_early_param+0x57/0x8e
> >>>> [ 0.000000] parse_args+0x208/0x320
> >>>> [ 0.000000] ? rdinit_setup+0x30/0x30
> >>>> [ 0.000000] parse_early_options+0x29/0x2d
> >>>> [ 0.000000] ? rdinit_setup+0x30/0x30
> >>>> [ 0.000000] parse_early_param+0x36/0x4d
> >>>> [ 0.000000] setup_arch+0x336/0x99e
> >>>> [ 0.000000] start_kernel+0x6f/0x4e6
> >>>> [ 0.000000] x86_64_start_reservations+0x24/0x26
> >>>> [ 0.000000] x86_64_start_kernel+0x6f/0x72
> >>>> [ 0.000000] secondary_startup_64+0xa4/0xb0
> >>>>
> >>>> This patch adds a check to prevent the panic.
> >>>>
> >>>> Signed-off-by: He Zhe <[email protected]>
> >>>> Cc: [email protected]
> >>>> Cc: [email protected]
> >>>> Cc: [email protected]
> >>>> Cc: [email protected]
> >>>> Cc: [email protected]
> >>>> ---
> >>>> kernel/dma/swiotlb.c | 5 +++++
> >>>> 1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> >>>> index 4f8a6db..46fc34e 100644
> >>>> --- a/kernel/dma/swiotlb.c
> >>>> +++ b/kernel/dma/swiotlb.c
> >>>> @@ -109,6 +109,11 @@ static int late_alloc;
> >>>> static int __init
> >>>> setup_io_tlb_npages(char *str)
> >>>> {
> >>>> + if (!str) {
> >>>> + pr_err("Config string not provided\n");
> >>>> + return -EINVAL;
> >>>> + }
> >>>> +
> >>>> if (isdigit(*str)) {
> >>>> io_tlb_nslabs = simple_strtoul(str, &str, 0);
> >>>> /* avoid tail segment of size < IO_TLB_SEGSIZE */
> >>> _______________________________________________
> >>> iommu mailing list
> >>> [email protected]
> >>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >
>

2018-12-03 10:25:16

by He Zhe

Subject: Re: [PATCH] kernel/dma: Fix panic caused by passing swiotlb to command line

On 2018/12/2 20:30, Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 30, 2018 at 07:06:10PM +0800, He Zhe wrote:
>>
>> On 2018/10/23 19:14, He Zhe wrote:
>>> On 2018/10/23 03:29, Konrad Rzeszutek Wilk wrote:
>>>> On Sat, Sep 22, 2018 at 08:56:58PM +0800, He Zhe wrote:
>>>>> May I have your input?
>>>> Alternatively would it make more sense for it to assume some default
>>>> value?
>>> Maybe, but the original code has no default value and I have no idea
>>> what default value is proper here.
>> Can anyone give some suggestions? Though I'd prefer to do nothing when
>> no option is provided.
> One provided the paramter for a reason. I would just do a small one, say
> 4MB.

OK. Thanks, I'll send v2.

Zhe

>> Thanks,
>> Zhe
>>
>>> Zhe
>>>
>>>>> Thanks,
>>>>> Zhe
>>>>>
>>>>> On 2018年09月17日 11:27, [email protected] wrote:
>>>>>> From: He Zhe <[email protected]>
>>>>>>
>>>>>> setup_io_tlb_npages does not check input argument before passing it
>>>>>> to isdigit. The argument would be a NULL pointer if "swiotlb", without
>>>>>> its value, is set in command line and thus causes the following panic.
>>>>>>
>>>>>> PANIC: early exception 0xe3 IP 10:ffffffffbb9b8e9f error 0 cr2 0x0
>>>>>> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc3-yocto-standard+ #9
>>>>>> [ 0.000000] RIP: 0010:setup_io_tlb_npages+0xf/0x95
>>>>>> ...
>>>>>> [ 0.000000] Call Trace:
>>>>>> [ 0.000000] do_early_param+0x57/0x8e
>>>>>> [ 0.000000] parse_args+0x208/0x320
>>>>>> [ 0.000000] ? rdinit_setup+0x30/0x30
>>>>>> [ 0.000000] parse_early_options+0x29/0x2d
>>>>>> [ 0.000000] ? rdinit_setup+0x30/0x30
>>>>>> [ 0.000000] parse_early_param+0x36/0x4d
>>>>>> [ 0.000000] setup_arch+0x336/0x99e
>>>>>> [ 0.000000] start_kernel+0x6f/0x4e6
>>>>>> [ 0.000000] x86_64_start_reservations+0x24/0x26
>>>>>> [ 0.000000] x86_64_start_kernel+0x6f/0x72
>>>>>> [ 0.000000] secondary_startup_64+0xa4/0xb0
>>>>>>
>>>>>> This patch adds a check to prevent the panic.
>>>>>>
>>>>>> Signed-off-by: He Zhe <[email protected]>
>>>>>> Cc: [email protected]
>>>>>> Cc: [email protected]
>>>>>> Cc: [email protected]
>>>>>> Cc: [email protected]
>>>>>> Cc: [email protected]
>>>>>> ---
>>>>>> kernel/dma/swiotlb.c | 5 +++++
>>>>>> 1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>>>>>> index 4f8a6db..46fc34e 100644
>>>>>> --- a/kernel/dma/swiotlb.c
>>>>>> +++ b/kernel/dma/swiotlb.c
>>>>>> @@ -109,6 +109,11 @@ static int late_alloc;
>>>>>> static int __init
>>>>>> setup_io_tlb_npages(char *str)
>>>>>> {
>>>>>> + if (!str) {
>>>>>> + pr_err("Config string not provided\n");
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> +
>>>>>> if (isdigit(*str)) {
>>>>>> io_tlb_nslabs = simple_strtoul(str, &str, 0);
>>>>>> /* avoid tail segment of size < IO_TLB_SEGSIZE */
>>>>> _______________________________________________
>>>>> iommu mailing list
>>>>> [email protected]
>>>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu