[PATCH] eventfd: Enlarge recursion limit to allow vhost to work
commit b5e683d5cab8 ("eventfd: track eventfd_signal() recursion depth") introduces a percpu counter that tracks the percpu recursion depth and warn if it greater than zero, to avoid potential deadlock and stack overflow. However sometimes different eventfds may be used in parallel. Specifically, when heavy network load goes through kvm and vhost, working as below, it would trigger the following call trace. - 100.00% - 66.51% ret_from_fork kthread - vhost_worker - 33.47% handle_tx_kick handle_tx handle_tx_copy vhost_tx_batch.isra.0 vhost_add_used_and_signal_n eventfd_signal - 33.05% handle_rx_net handle_rx vhost_add_used_and_signal_n eventfd_signal - 33.49% ioctl entry_SYSCALL_64_after_hwframe do_syscall_64 __x64_sys_ioctl ksys_ioctl do_vfs_ioctl kvm_vcpu_ioctl kvm_arch_vcpu_ioctl_run vmx_handle_exit handle_ept_misconfig kvm_io_bus_write __kvm_io_bus_write eventfd_signal 001: WARNING: CPU: 1 PID: 1503 at fs/eventfd.c:73 eventfd_signal+0x85/0xa0 snip 001: Call Trace: 001: vhost_signal+0x15e/0x1b0 [vhost] 001: vhost_add_used_and_signal_n+0x2b/0x40 [vhost] 001: handle_rx+0xb9/0x900 [vhost_net] 001: handle_rx_net+0x15/0x20 [vhost_net] 001: vhost_worker+0xbe/0x120 [vhost] 001: kthread+0x106/0x140 001: ? log_used.part.0+0x20/0x20 [vhost] 001: ? kthread_park+0x90/0x90 001: ret_from_fork+0x35/0x40 001: ---[ end trace 0003 ]--- This patch enlarges the limit to 1 which is the maximum recursion depth we have found so far. The credit of modification for eventfd_signal_count goes to Xie Yongji Signed-off-by: He Zhe --- fs/eventfd.c| 3 ++- include/linux/eventfd.h | 5 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/eventfd.c b/fs/eventfd.c index e265b6dd4f34..add6af91cacf 100644 --- a/fs/eventfd.c +++ b/fs/eventfd.c @@ -71,7 +71,8 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n) * it returns true, the eventfd_signal() call should be deferred to a * safe context. */ - if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count))) + if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count) > + EFD_WAKE_COUNT_MAX)) return 0; spin_lock_irqsave(>wqh.lock, flags); diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h index fa0a524baed0..74be152ebe87 100644 --- a/include/linux/eventfd.h +++ b/include/linux/eventfd.h @@ -29,6 +29,9 @@ #define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK) #define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE) +/* This is the maximum recursion depth we find so far */ +#define EFD_WAKE_COUNT_MAX 1 + struct eventfd_ctx; struct file; @@ -47,7 +50,7 @@ DECLARE_PER_CPU(int, eventfd_wake_count); static inline bool eventfd_signal_count(void) { - return this_cpu_read(eventfd_wake_count); + return this_cpu_read(eventfd_wake_count) > EFD_WAKE_COUNT_MAX; } #else /* CONFIG_EVENTFD */ -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 03/10] eventfd: Increase the recursion depth of eventfd_signal()
On 6/18/21 11:29 AM, Yongji Xie wrote: > On Thu, Jun 17, 2021 at 4:34 PM He Zhe wrote: >> >> >> On 6/15/21 10:13 PM, Xie Yongji wrote: >>> Increase the recursion depth of eventfd_signal() to 1. This >>> is the maximum recursion depth we have found so far, which >>> can be triggered with the following call chain: >>> >>> kvm_io_bus_write[kvm] >>> --> ioeventfd_write [kvm] >>> --> eventfd_signal [eventfd] >>> --> vhost_poll_wakeup [vhost] >>> --> vduse_vdpa_kick_vq [vduse] >>> --> eventfd_signal[eventfd] >>> >>> Signed-off-by: Xie Yongji >>> Acked-by: Jason Wang >> The fix had been posted one year ago. >> >> https://lore.kernel.org/lkml/20200410114720.24838-1-zhe...@windriver.com/ >> > OK, so it seems to be a fix for the RT system if my understanding is > correct? Any reason why it's not merged? I'm happy to rebase my series > on your patch if you'd like to repost it. It works for both mainline and RT kernel. The folks just reproduced in their RT environments. This patch somehow hasn't got maintainer's reply, so not merged yet. And OK, I'll resend the patch. > > BTW, I also notice another thread for this issue: > > https://lore.kernel.org/linux-fsdevel/dm6pr11mb420291b550a10853403c7592ff...@dm6pr11mb4202.namprd11.prod.outlook.com/T/ This is the same way as my v1 https://lore.kernel.org/lkml/3b4aa4cb-0e76-89c2-c48a-cf24e1a36...@kernel.dk/ which was not what the maintainer wanted. > >>> --- >>> fs/eventfd.c| 2 +- >>> include/linux/eventfd.h | 5 - >>> 2 files changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/eventfd.c b/fs/eventfd.c >>> index e265b6dd4f34..cc7cd1dbedd3 100644 >>> --- a/fs/eventfd.c >>> +++ b/fs/eventfd.c >>> @@ -71,7 +71,7 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n) >>>* it returns true, the eventfd_signal() call should be deferred to a >>>* safe context. >>>*/ >>> - if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count))) >>> + if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count) > EFD_WAKE_DEPTH)) >>> return 0; >>> >>> spin_lock_irqsave(>wqh.lock, flags); >>> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h >>> index fa0a524baed0..886d99cd38ef 100644 >>> --- a/include/linux/eventfd.h >>> +++ b/include/linux/eventfd.h >>> @@ -29,6 +29,9 @@ >>> #define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK) >>> #define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE) >>> >>> +/* Maximum recursion depth */ >>> +#define EFD_WAKE_DEPTH 1 >>> + >>> struct eventfd_ctx; >>> struct file; >>> >>> @@ -47,7 +50,7 @@ DECLARE_PER_CPU(int, eventfd_wake_count); >>> >>> static inline bool eventfd_signal_count(void) >>> { >>> - return this_cpu_read(eventfd_wake_count); >>> + return this_cpu_read(eventfd_wake_count) > EFD_WAKE_DEPTH; >> count is just count. How deep is acceptable should be put >> where eventfd_signal_count is called. >> > The return value of this function is boolean rather than integer. > Please see the comments in eventfd_signal(): > > "then it should check eventfd_signal_count() before calling this > function. If it returns true, the eventfd_signal() call should be > deferred to a safe context." OK. Now that the maintainer comments as such we can use it accordingly, though I still got the feeling that the function name and the type of the return value don't match. Thanks, Zhe > > Thanks, > Yongji ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 03/10] eventfd: Increase the recursion depth of eventfd_signal()
On 6/15/21 10:13 PM, Xie Yongji wrote: > Increase the recursion depth of eventfd_signal() to 1. This > is the maximum recursion depth we have found so far, which > can be triggered with the following call chain: > > kvm_io_bus_write[kvm] > --> ioeventfd_write [kvm] > --> eventfd_signal [eventfd] > --> vhost_poll_wakeup [vhost] > --> vduse_vdpa_kick_vq [vduse] > --> eventfd_signal[eventfd] > > Signed-off-by: Xie Yongji > Acked-by: Jason Wang The fix had been posted one year ago. https://lore.kernel.org/lkml/20200410114720.24838-1-zhe...@windriver.com/ > --- > fs/eventfd.c| 2 +- > include/linux/eventfd.h | 5 - > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/eventfd.c b/fs/eventfd.c > index e265b6dd4f34..cc7cd1dbedd3 100644 > --- a/fs/eventfd.c > +++ b/fs/eventfd.c > @@ -71,7 +71,7 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n) >* it returns true, the eventfd_signal() call should be deferred to a >* safe context. >*/ > - if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count))) > + if (WARN_ON_ONCE(this_cpu_read(eventfd_wake_count) > EFD_WAKE_DEPTH)) > return 0; > > spin_lock_irqsave(>wqh.lock, flags); > diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h > index fa0a524baed0..886d99cd38ef 100644 > --- a/include/linux/eventfd.h > +++ b/include/linux/eventfd.h > @@ -29,6 +29,9 @@ > #define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK) > #define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE) > > +/* Maximum recursion depth */ > +#define EFD_WAKE_DEPTH 1 > + > struct eventfd_ctx; > struct file; > > @@ -47,7 +50,7 @@ DECLARE_PER_CPU(int, eventfd_wake_count); > > static inline bool eventfd_signal_count(void) > { > - return this_cpu_read(eventfd_wake_count); > + return this_cpu_read(eventfd_wake_count) > EFD_WAKE_DEPTH; count is just count. How deep is acceptable should be put where eventfd_signal_count is called. Zhe > } > > #else /* CONFIG_EVENTFD */ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line
On 1/11/19 9:46 AM, He Zhe wrote: > > On 1/11/19 1:27 AM, Konrad Rzeszutek Wilk wrote: >> Let's skip it. There was another patch that would allocate a default 4MB >> size if it there was an misue of swiotlb parameters. > But this patch mainly fixes a crash. Could you please point me to the patch > you mentioned? And the v2 is modified according to your suggestion. Zhe > > Thanks, > Zhe > >> >> >> On Mon, Jan 7, 2019, 4:07 AM Christoph Hellwig > <mailto:h...@lst.de> wrote: >> >> On Mon, Jan 07, 2019 at 04:46:51PM +0800, He Zhe wrote: >> > Kindly ping. >> >> Konrad, I'll pick this up through the DMA mapping tree unless you >> protest in the next few days. >> > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line
On 1/11/19 1:27 AM, Konrad Rzeszutek Wilk wrote: > Let's skip it. There was another patch that would allocate a default 4MB size > if it there was an misue of swiotlb parameters. But this patch mainly fixes a crash. Could you please point me to the patch you mentioned? Thanks, Zhe > > > > On Mon, Jan 7, 2019, 4:07 AM Christoph Hellwig <mailto:h...@lst.de> wrote: > > On Mon, Jan 07, 2019 at 04:46:51PM +0800, He Zhe wrote: > > Kindly ping. > > Konrad, I'll pick this up through the DMA mapping tree unless you > protest in the next few days. > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] kernel/dma: Fix panic caused by passing swiotlb to command line
Kindly ping. Zhe On 12/3/18 6:22 PM, zhe...@windriver.com wrote: > From: He Zhe > > 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:bb9b8e9f error 0 cr2 0x0 > [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted > 4.19.0-rc3-yocto-standard+ #9 > [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95 > ... > [0.00] Call Trace: > [0.00] do_early_param+0x57/0x8e > [0.00] parse_args+0x208/0x320 > [0.00] ? rdinit_setup+0x30/0x30 > [0.00] parse_early_options+0x29/0x2d > [0.00] ? rdinit_setup+0x30/0x30 > [0.00] parse_early_param+0x36/0x4d > [0.00] setup_arch+0x336/0x99e > [0.00] start_kernel+0x6f/0x4e6 > [0.00] x86_64_start_reservations+0x24/0x26 > [0.00] x86_64_start_kernel+0x6f/0x72 > [0.00] secondary_startup_64+0xa4/0xb0 > > This patch adds a check to prevent the panic and sets it for 4MB by default. > > Signed-off-by: He Zhe > Cc: sta...@vger.kernel.org > Cc: konrad.w...@oracle.com > Cc: h...@lst.de > Cc: m.szyprow...@samsung.com > Cc: robin.mur...@arm.com > > Signed-off-by: He Zhe > --- > v2: Set swiotlb for 4MB by default > > kernel/dma/swiotlb.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c > index 045930e..0e18cd4 100644 > --- a/kernel/dma/swiotlb.c > +++ b/kernel/dma/swiotlb.c > @@ -58,6 +58,9 @@ > */ > #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT) > > +#define IO_TLB_DEFAULT_MB 4 > +#define IO_TLB_DEFAULT_SLABS ((IO_TLB_DEFAULT_MB<<20) >> IO_TLB_SHIFT) > + > enum swiotlb_force swiotlb_force; > > /* > @@ -103,6 +106,14 @@ static int late_alloc; > static int __init > setup_io_tlb_npages(char *str) > { > + if (!str) { > + pr_err("No value provided for swiotlb, " > +"set it to %d slabs for %dMB by default.\n", > +IO_TLB_DEFAULT_SLABS, IO_TLB_DEFAULT_MB); > + io_tlb_nslabs = IO_TLB_DEFAULT_SLABS; > + return 0; > + } > + > if (isdigit(*str)) { > io_tlb_nslabs = simple_strtoul(str, , 0); > /* avoid tail segment of size < IO_TLB_SEGSIZE */ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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, zhe...@windriver.com wrote: >>>>>> From: He Zhe >>>>>> >>>>>> 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:bb9b8e9f error 0 cr2 0x0 >>>>>> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted >>>>>> 4.19.0-rc3-yocto-standard+ #9 >>>>>> [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95 >>>>>> ... >>>>>> [0.00] Call Trace: >>>>>> [0.00] do_early_param+0x57/0x8e >>>>>> [0.00] parse_args+0x208/0x320 >>>>>> [0.00] ? rdinit_setup+0x30/0x30 >>>>>> [0.00] parse_early_options+0x29/0x2d >>>>>> [0.00] ? rdinit_setup+0x30/0x30 >>>>>> [0.00] parse_early_param+0x36/0x4d >>>>>> [0.00] setup_arch+0x336/0x99e >>>>>> [0.00] start_kernel+0x6f/0x4e6 >>>>>> [0.00] x86_64_start_reservations+0x24/0x26 >>>>>> [0.00] x86_64_start_kernel+0x6f/0x72 >>>>>> [0.00] secondary_startup_64+0xa4/0xb0 >>>>>> >>>>>> This patch adds a check to prevent the panic. >>>>>> >>>>>> Signed-off-by: He Zhe >>>>>> Cc: sta...@vger.kernel.org >>>>>> Cc: konrad.w...@oracle.com >>>>>> Cc: h...@lst.de >>>>>> Cc: m.szyprow...@samsung.com >>>>>> Cc: robin.mur...@arm.com >>>>>> --- >>>>>> 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, , 0); >>>>>> /* avoid tail segment of size < IO_TLB_SEGSIZE */ >>>>> ___ >>>>> iommu mailing list >>>>> iommu@lists.linux-foundation.org >>>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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, zhe...@windriver.com wrote: >>>> From: He Zhe >>>> >>>> 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:bb9b8e9f error 0 cr2 0x0 >>>> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted >>>> 4.19.0-rc3-yocto-standard+ #9 >>>> [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95 >>>> ... >>>> [0.00] Call Trace: >>>> [0.00] do_early_param+0x57/0x8e >>>> [0.00] parse_args+0x208/0x320 >>>> [0.00] ? rdinit_setup+0x30/0x30 >>>> [0.00] parse_early_options+0x29/0x2d >>>> [0.00] ? rdinit_setup+0x30/0x30 >>>> [0.00] parse_early_param+0x36/0x4d >>>> [ 0.00] setup_arch+0x336/0x99e >>>> [0.00] start_kernel+0x6f/0x4e6 >>>> [0.00] x86_64_start_reservations+0x24/0x26 >>>> [0.00] x86_64_start_kernel+0x6f/0x72 >>>> [0.00] secondary_startup_64+0xa4/0xb0 >>>> >>>> This patch adds a check to prevent the panic. >>>> >>>> Signed-off-by: He Zhe >>>> Cc: sta...@vger.kernel.org >>>> Cc: konrad.w...@oracle.com >>>> Cc: h...@lst.de >>>> Cc: m.szyprow...@samsung.com >>>> Cc: robin.mur...@arm.com >>>> --- >>>> 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, , 0); >>>>/* avoid tail segment of size < IO_TLB_SEGSIZE */ >>> ___ >>> iommu mailing list >>> iommu@lists.linux-foundation.org >>> https://lists.linuxfoundation.org/mailman/listinfo/iommu > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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, zhe...@windriver.com wrote: >>> From: He Zhe >>> >>> 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:bb9b8e9f error 0 cr2 0x0 >>> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted >>> 4.19.0-rc3-yocto-standard+ #9 >>> [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95 >>> ... >>> [0.00] Call Trace: >>> [0.00] do_early_param+0x57/0x8e >>> [0.00] parse_args+0x208/0x320 >>> [0.00] ? rdinit_setup+0x30/0x30 >>> [0.00] parse_early_options+0x29/0x2d >>> [0.00] ? rdinit_setup+0x30/0x30 >>> [0.00] parse_early_param+0x36/0x4d >>> [0.00] setup_arch+0x336/0x99e >>> [0.00] start_kernel+0x6f/0x4e6 >>> [0.00] x86_64_start_reservations+0x24/0x26 >>> [0.00] x86_64_start_kernel+0x6f/0x72 >>> [0.00] secondary_startup_64+0xa4/0xb0 >>> >>> This patch adds a check to prevent the panic. >>> >>> Signed-off-by: He Zhe >>> Cc: sta...@vger.kernel.org >>> Cc: konrad.w...@oracle.com >>> Cc: h...@lst.de >>> Cc: m.szyprow...@samsung.com >>> Cc: robin.mur...@arm.com >>> --- >>> 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, , 0); >>> /* avoid tail segment of size < IO_TLB_SEGSIZE */ >> ___ >> iommu mailing list >> iommu@lists.linux-foundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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, zhe...@windriver.com wrote: > From: He Zhe > > 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:bb9b8e9f error 0 cr2 0x0 > [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted > 4.19.0-rc3-yocto-standard+ #9 > [0.00] RIP: 0010:setup_io_tlb_npages+0xf/0x95 > ... > [0.00] Call Trace: > [0.00] do_early_param+0x57/0x8e > [0.00] parse_args+0x208/0x320 > [0.00] ? rdinit_setup+0x30/0x30 > [0.00] parse_early_options+0x29/0x2d > [0.00] ? rdinit_setup+0x30/0x30 > [0.00] parse_early_param+0x36/0x4d > [0.00] setup_arch+0x336/0x99e > [0.00] start_kernel+0x6f/0x4e6 > [0.00] x86_64_start_reservations+0x24/0x26 > [0.00] x86_64_start_kernel+0x6f/0x72 > [0.00] secondary_startup_64+0xa4/0xb0 > > This patch adds a check to prevent the panic. > > Signed-off-by: He Zhe > Cc: sta...@vger.kernel.org > Cc: konrad.w...@oracle.com > Cc: h...@lst.de > Cc: m.szyprow...@samsung.com > Cc: robin.mur...@arm.com > --- > 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, , 0); > /* avoid tail segment of size < IO_TLB_SEGSIZE */ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu