Re: [RFC PATCH 03/12] staging: android: ion: Duplicate sg_table
On March 03, 2017 5:45 AM Laura Abbott wrote:
>
> +static struct sg_table *dup_sg_table(struct sg_table *table)
> +{
> + struct sg_table *new_table;
> + int ret, i;
> + struct scatterlist *sg, *new_sg;
> +
> + new_table = kzalloc(sizeof(*new_table), GFP_KERNEL);
> + if (!new_table)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
> + if (ret) {
> + kfree(table);
Free new table?
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + new_sg = new_table->sgl;
> + for_each_sg(table->sgl, sg, table->nents, i) {
> + memcpy(new_sg, sg, sizeof(*sg));
> + sg->dma_address = 0;
> + new_sg = sg_next(new_sg);
> + }
> +
Do we need a helper, sg_copy_table(dst_table, src_table)?
> + return new_table;
> +}
> +
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: KASAN: slab-out-of-bounds Read in hfa384x_usbin_callback
On Wed, 25 Mar 2020 01:58:03 -0700 > syzbot has tested the proposed patch but the reproducer still triggered crash: > KASAN: use-after-free Read in hfa384x_usbin_callback > > == > BUG: KASAN: use-after-free in memcpy include/linux/string.h:381 [inline] > BUG: KASAN: use-after-free in skb_put_data include/linux/skbuff.h:2284 > [inline] > BUG: KASAN: use-after-free in hfa384x_int_rxmonitor > drivers/staging/wlan-ng/hfa384x_usb.c:3412 [inline] > BUG: KASAN: use-after-free in hfa384x_usbin_rx > drivers/staging/wlan-ng/hfa384x_usb.c:3312 [inline] > BUG: KASAN: use-after-free in hfa384x_usbin_callback+0x1993/0x2360 > drivers/staging/wlan-ng/hfa384x_usb.c:3026 > Read of size 19671 at addr 8881cda7b33c by task kworker/1:2/95 > > CPU: 1 PID: 95 Comm: kworker/1:2 Not tainted 5.6.0-rc5-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Workqueue: usb_hub_wq hub_event > Call Trace: > > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0xef/0x16e lib/dump_stack.c:118 > print_address_description.constprop.0.cold+0xd3/0x314 mm/kasan/report.c:374 > __kasan_report.cold+0x37/0x77 mm/kasan/report.c:506 > kasan_report+0xe/0x20 mm/kasan/common.c:641 > check_memory_region_inline mm/kasan/generic.c:185 [inline] > check_memory_region+0x152/0x1c0 mm/kasan/generic.c:192 > memcpy+0x20/0x50 mm/kasan/common.c:127 > memcpy include/linux/string.h:381 [inline] > skb_put_data include/linux/skbuff.h:2284 [inline] > hfa384x_int_rxmonitor drivers/staging/wlan-ng/hfa384x_usb.c:3412 [inline] > hfa384x_usbin_rx drivers/staging/wlan-ng/hfa384x_usb.c:3312 [inline] > hfa384x_usbin_callback+0x1993/0x2360 > drivers/staging/wlan-ng/hfa384x_usb.c:3026 > __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650 > usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716 > dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966 > call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404 > expire_timers kernel/time/timer.c:1449 [inline] > __run_timers kernel/time/timer.c:1773 [inline] > __run_timers kernel/time/timer.c:1740 [inline] > run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786 > __do_softirq+0x21e/0x950 kernel/softirq.c:292 > invoke_softirq kernel/softirq.c:373 [inline] > irq_exit+0x178/0x1a0 kernel/softirq.c:413 > exiting_irq arch/x86/include/asm/apic.h:546 [inline] > smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146 > apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829 > > RIP: 0010:arch_local_irq_restore arch/x86/include/asm/irqflags.h:85 [inline] > RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 > [inline] > RIP: 0010:_raw_spin_unlock_irqrestore+0x3b/0x40 kernel/locking/spinlock.c:191 > Code: e8 2a e8 96 fb 48 89 ef e8 f2 c9 97 fb f6 c7 02 75 11 53 9d e8 16 50 b5 > fb 65 ff 0d f7 bd 72 7a 5b 5d c3 e8 07 4e b5 fb 53 9d ed 0f 1f 00 55 48 > 89 fd 65 ff 05 dd bd 72 7a 45 31 c9 41 b8 01 > RSP: 0018:8881d56b6f40 EFLAGS: 0293 ORIG_RAX: ff13 > RAX: 0007 RBX: 0293 RCX: 0006 > RDX: RSI: 8881d56a88f0 RDI: 8881d56a884c > RBP: 8881c0c64b80 R08: 8881d56a8000 R09: fbfff1266e8f > R10: fbfff1266e8e R11: 89337477 R12: > R13: 8881c0c64bb8 R14: 8881c0c64b80 R15: 8881c0c64bb8 > hfa384x_usbctlx_submit+0x1cb/0x260 drivers/staging/wlan-ng/hfa384x_usb.c:3834 > hfa384x_docmd drivers/staging/wlan-ng/hfa384x_usb.c:1233 [inline] > hfa384x_cmd_initialize+0x290/0x4f0 drivers/staging/wlan-ng/hfa384x_usb.c:846 > hfa384x_drvr_start+0x1f1/0x480 drivers/staging/wlan-ng/hfa384x_usb.c:2380 > prism2sta_ifstate+0x24e/0x510 drivers/staging/wlan-ng/prism2sta.c:471 > prism2sta_probe_usb.cold+0x1c8/0x49e drivers/staging/wlan-ng/prism2usb.c:112 > usb_probe_interface+0x310/0x800 drivers/usb/core/driver.c:374 > really_probe+0x290/0xac0 drivers/base/dd.c:551 > driver_probe_device+0x223/0x350 drivers/base/dd.c:724 > __device_attach_driver+0x1d1/0x290 drivers/base/dd.c:831 > bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:431 > __device_attach+0x217/0x390 drivers/base/dd.c:897 > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491 > device_add+0x1459/0x1bf0 drivers/base/core.c:2500 > usb_set_configuration+0xe47/0x17d0 drivers/usb/core/message.c:2023 > usb_generic_driver_probe+0x9d/0xe0 drivers/usb/core/generic.c:241 > usb_probe_device+0xd9/0x230 drivers/usb/core/driver.c:272 > really_probe+0x290/0xac0 drivers/base/dd.c:551 > driver_probe_device+0x223/0x350 drivers/base/dd.c:724 > __device_attach_driver+0x1d1/0x290 drivers/base/dd.c:831 > bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:431 > __device_attach+0x217/0x390 drivers/base/dd.c:897 > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491 > device_add+0x1459/0x1bf0 drivers/base/core.c:2500 > usb_new_device.cold+0x540/0xcd0 drivers/usb/core
Re: [PATCH] arch:arm:mm:Correction in the boundary check for module end address.
> On Tue, Nov 10, 2015 at 12:35:50AM +0900, Jungseung Lee wrote:
> > Hi,
> >
> > 2015-11-09 16:57 GMT+09:00 Shailendra Verma :
> > > From: Shailendra Verma
> > >
> > > The module end boundary check is not proper.The out of bound value
> > > of module end can produce undesired results.
> > >
> > > Signed-off-by: Shailendra Verma
> > > Reviewed-by: Ravikant Bijendra Sharma
> > > ---
> > > linux-4.3-rc6/arch/arm/mm/pageattr.c |2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/linux-4.3-rc6/arch/arm/mm/pageattr.c
> > > b/linux-4.3-rc6/arch/arm/mm/pageattr.c
> > > index cf30daf..be7fe4b 100644
> > > --- a/linux-4.3-rc6/arch/arm/mm/pageattr.c
> > > +++ b/linux-4.3-rc6/arch/arm/mm/pageattr.c
> > > @@ -52,7 +52,7 @@ static int change_memory_common(unsigned long addr, int
> > > numpages,
> > > if (start < MODULES_VADDR || start >= MODULES_END)
> > > return -EINVAL;
> > >
> > > - if (end < MODULES_VADDR || start >= MODULES_END)
> > > + if (end < MODULES_VADDR || end >= MODULES_END)
> > > return -EINVAL;
> > >
> > > data.set_mask = set_mask;
> > > --
> > > 1.7.9.5
> > >
> > Same patch with proper format is already submitted by Hillf.
> > https://lkml.org/lkml/2015/5/3/202
>
> What happened to that patch? It should at least have had a:
>
> Fixes: f2ca09f381a5 ("ARM: 8311/1: Don't use is_module_addr in setting page
> attributes")
>
> tag on it, and it needs to find its way into the patch system. As
> no one has reported a crash (I don't think it's crash-causing, but is
> merely a correctness issue) I don't see any reason to backport it to
> stable trees.
>
> Other changes are needed here as well - the original commit creating
> this contains:
>
> + if (!IS_ALIGNED(addr, PAGE_SIZE)) {
> + start &= PAGE_MASK;
> + end = start + size;
> + WARN_ON_ONCE(1);
> + }
>
> which is truely amazing. Fine, it may not be intended to work with
> non-aligned addresses, but if we're going to round the start down,
> then rounding the end down as well like that is also buggy.
>
> unsigned long start = addr;
> unsigned long size = PAGE_SIZE * numpages;
> unsigned long end = start + size;
>
> if (WARN_ON_ONCE(!IS_ALIGNED(addr, PAGE_SIZE)) {
> start &= PAGE_MASK;
> end = PAGE_ALIGN(end);
> }
>
> would be much better - this will round 'end' up, so we encompass the
> entire requested region.
>
Would you please respin, Shailendra, with Russell's comments
addressed, and with
Reported-by: Hillf Danton ?
thanks
Hillf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC 6/6] drivers: staging: ion: add ION_IOC_TAG ioctl
On Wednesday, October 12, 2016 7:50 AM Ruchi Kandoi wrote:
> +/**
> + * struct ion_fd_data - metadata passed from userspace for a handle
s/fd/tag/ ?
> + * @handle: a handle
> + * @tag: a string describing the buffer
> + *
> + * For ION_IOC_TAG userspace populates the handle field with
> + * the handle returned from ion alloc and type contains the memtrack_type
> which
> + * accurately describes the usage for the memory.
> + */
> +struct ion_tag_data {
> + ion_user_handle_t handle;
> + char tag[ION_MAX_TAG_LEN];
> +};
> +
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch v2] Documentation/email-clients.txt: add a section about git
On Thu, May 8, 2014 at 5:05 PM, Julian Andres Klode wrote: > On Thu, May 08, 2014 at 11:44:12AM +0300, Dan Carpenter wrote: >> +as raw text including all the headers. Run `cat raw_email.txt | git am` > > `cat raw_email.txt | git am` seems a bit pointless. Why not simply > `git am raw_email.txt`? `git am < raw_email.txt` does the same too > and avoids the useless cat as well. > > No point in running two processes when the same can be done in a > simpler way in one process, IMO. > And this helps most developers little, but is more valuable for those who dont use 'git send-mail' everyday if the detail of cmdline is provided, say send a patch file with 'git send-mail' through gmail. We will say double thanks to you, Dan, if you really help us tame gmail. And a howto to tame gmail is doule apprecaited. thanks Hillf ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: possible deadlock in shmem_fallocate (4)
Mon, 13 Jul 2020 17:32:19 -0700
> syzbot has found a reproducer for the following crash on:
>
> HEAD commit:11ba4688 Linux 5.8-rc5
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=13f1bf4710
> kernel config: https://syzkaller.appspot.com/x/.config?x=a160d1053fc89af5
> dashboard link: https://syzkaller.appspot.com/bug?extid=7a0d9d0b26efefe61780
> compiler: gcc (GCC) 10.1.0-syz 20200507
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1181004f10
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+7a0d9d0b26efefe61...@syzkaller.appspotmail.com
>
> ==
> WARNING: possible circular locking dependency detected
> 5.8.0-rc5-syzkaller #0 Not tainted
> --
> khugepaged/1157 is trying to acquire lock:
> 88809272e910 (&sb->s_type->i_mutex_key#15){+.+.}-{3:3}, at: inode_lock
> include/linux/fs.h:800 [inline]
> 88809272e910 (&sb->s_type->i_mutex_key#15){+.+.}-{3:3}, at:
> shmem_fallocate+0x153/0xd90 mm/shmem.c:2707
>
> but task is already holding lock:
> 89c6c260 (fs_reclaim){+.+.}-{0:0}, at: fs_reclaim_release
> mm/page_alloc.c:4202 [inline]
> 89c6c260 (fs_reclaim){+.+.}-{0:0}, at: fs_reclaim_release
> mm/page_alloc.c:4198 [inline]
> 89c6c260 (fs_reclaim){+.+.}-{0:0}, at: __perform_reclaim
> mm/page_alloc.c:4227 [inline]
> 89c6c260 (fs_reclaim){+.+.}-{0:0}, at: __alloc_pages_direct_reclaim
> mm/page_alloc.c:4244 [inline]
> 89c6c260 (fs_reclaim){+.+.}-{0:0}, at:
> __alloc_pages_slowpath.constprop.0+0x1554/0x2780 mm/page_alloc.c:4650
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (fs_reclaim){+.+.}-{0:0}:
>__fs_reclaim_acquire mm/page_alloc.c:4183 [inline]
>fs_reclaim_acquire mm/page_alloc.c:4194 [inline]
>prepare_alloc_pages mm/page_alloc.c:4780 [inline]
>__alloc_pages_nodemask+0x3d1/0x930 mm/page_alloc.c:4832
>alloc_pages_vma+0xdd/0x720 mm/mempolicy.c:2255
>shmem_alloc_page+0x11f/0x1f0 mm/shmem.c:1502
>shmem_alloc_and_acct_page+0x161/0x8a0 mm/shmem.c:1527
>shmem_getpage_gfp+0x511/0x2450 mm/shmem.c:1823
>shmem_getpage mm/shmem.c:153 [inline]
>shmem_write_begin+0xf9/0x1d0 mm/shmem.c:2459
>generic_perform_write+0x20a/0x4f0 mm/filemap.c:3318
>__generic_file_write_iter+0x24b/0x610 mm/filemap.c:3447
>generic_file_write_iter+0x3a6/0x5c0 mm/filemap.c:3479
>call_write_iter include/linux/fs.h:1908 [inline]
>new_sync_write+0x422/0x650 fs/read_write.c:503
>vfs_write+0x59d/0x6b0 fs/read_write.c:578
>ksys_write+0x12d/0x250 fs/read_write.c:631
>do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:384
>entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> -> #0 (&sb->s_type->i_mutex_key#15){+.+.}-{3:3}:
>check_prev_add kernel/locking/lockdep.c:2496 [inline]
>check_prevs_add kernel/locking/lockdep.c:2601 [inline]
>validate_chain kernel/locking/lockdep.c:3218 [inline]
>__lock_acquire+0x2acb/0x56e0 kernel/locking/lockdep.c:4380
>lock_acquire+0x1f1/0xad0 kernel/locking/lockdep.c:4959
>down_write+0x8d/0x150 kernel/locking/rwsem.c:1531
>inode_lock include/linux/fs.h:800 [inline]
>shmem_fallocate+0x153/0xd90 mm/shmem.c:2707
>ashmem_shrink_scan.part.0+0x2e9/0x490
> drivers/staging/android/ashmem.c:490
>ashmem_shrink_scan+0x6c/0xa0 drivers/staging/android/ashmem.c:473
>do_shrink_slab+0x3c6/0xab0 mm/vmscan.c:518
>shrink_slab+0x16f/0x5c0 mm/vmscan.c:679
>shrink_node_memcgs mm/vmscan.c:2658 [inline]
>shrink_node+0x519/0x1b60 mm/vmscan.c:2770
>shrink_zones mm/vmscan.c:2973 [inline]
>do_try_to_free_pages+0x38b/0x1340 mm/vmscan.c:3026
>try_to_free_pages+0x29a/0x8b0 mm/vmscan.c:3265
>__perform_reclaim mm/page_alloc.c:4223 [inline]
>__alloc_pages_direct_reclaim mm/page_alloc.c:4244 [inline]
>__alloc_pages_slowpath.constprop.0+0x949/0x2780 mm/page_alloc.c:4650
>__alloc_pages_nodemask+0x68f/0x930 mm/page_alloc.c:4863
>__alloc_pages include/linux/gfp.h:509 [inline]
>__alloc_pages_node include/linux/gfp.h:522 [inline]
>khugepaged_alloc_page+0xa0/0x170 mm/khugepaged.c:867
>collapse_huge_page mm/khugepaged.c:1056 [inline]
>khugepaged_scan_pmd mm/khugepaged.c:1349 [inline]
>khugepaged_scan_mm_slot mm/khugepaged.c:2110 [inline]
>khugepaged_do_scan mm/khugepaged.c:2193 [inline]
>khugepaged+0x3093/0x5a10 mm/khugepaged.c:2238
>kthread+0x3b5/0x4a0 kernel/kthread.c:291
>ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
>CPU0
Re: possible deadlock in shmem_fallocate (4)
On Mon, 13 Jul 2020 20:41:11 -0700 Eric Biggers wrote:
> On Tue, Jul 14, 2020 at 11:32:52AM +0800, Hillf Danton wrote:
> >
> > Add FALLOC_FL_NOBLOCK and on the shmem side try to lock inode upon the
> > new flag. And the overall upside is to keep the current gfp either in
> > the khugepaged context or not.
> >
> > --- a/include/uapi/linux/falloc.h
> > +++ b/include/uapi/linux/falloc.h
> > @@ -77,4 +77,6 @@
> > */
> > #define FALLOC_FL_UNSHARE_RANGE0x40
> >
> > +#define FALLOC_FL_NOBLOCK 0x80
> > +
>
> You can't add a new UAPI flag to fix a kernel-internal problem like this.
Sounds fair, see below.
What the report indicates is a missing PF_MEMALLOC_NOFS and it's
checked on the ashmem side and added as an exception before going
to filesystem. On shmem side, no more than a best effort is paid
on the inteded exception.
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -437,6 +437,7 @@ static unsigned long
ashmem_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
{
unsigned long freed = 0;
+ bool nofs;
/* We might recurse into filesystem code, so bail out if necessary */
if (!(sc->gfp_mask & __GFP_FS))
@@ -445,6 +446,11 @@ ashmem_shrink_scan(struct shrinker *shri
if (!mutex_trylock(&ashmem_mutex))
return -1;
+ /* enter filesystem with caution: nonblock on locking */
+ nofs = current->flags & PF_MEMALLOC_NOFS;
+ if (!nofs)
+ current->flags |= PF_MEMALLOC_NOFS;
+
while (!list_empty(&ashmem_lru_list)) {
struct ashmem_range *range =
list_first_entry(&ashmem_lru_list, typeof(*range), lru);
@@ -472,6 +478,8 @@ ashmem_shrink_scan(struct shrinker *shri
}
mutex_unlock(&ashmem_mutex);
out:
+ if (!nofs)
+ current->flags &= ~PF_MEMALLOC_NOFS;
return freed;
}
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2728,7 +2728,12 @@ static long shmem_fallocate(struct file
if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
return -EOPNOTSUPP;
- inode_lock(inode);
+ if (current->flags & PF_MEMALLOC_NOFS) {
+ /* this exception needs a best effort and no more */
+ if (!inode_trylock(inode))
+ return -EBUSY;
+ } else
+ inode_lock(inode);
if (mode & FALLOC_FL_PUNCH_HOLE) {
struct address_space *mapping = file->f_mapping;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: possible deadlock in shmem_fallocate (4)
On Tue, 14 Jul 2020 10:26:29 +0200 Michal Hocko wrote:
> On Tue 14-07-20 13:32:05, Hillf Danton wrote:
> >
> > On Mon, 13 Jul 2020 20:41:11 -0700 Eric Biggers wrote:
> > > On Tue, Jul 14, 2020 at 11:32:52AM +0800, Hillf Danton wrote:
> > > >
> > > > Add FALLOC_FL_NOBLOCK and on the shmem side try to lock inode upon the
> > > > new flag. And the overall upside is to keep the current gfp either in
> > > > the khugepaged context or not.
> > > >
> > > > --- a/include/uapi/linux/falloc.h
> > > > +++ b/include/uapi/linux/falloc.h
> > > > @@ -77,4 +77,6 @@
> > > > */
> > > > #define FALLOC_FL_UNSHARE_RANGE0x40
> > > >
> > > > +#define FALLOC_FL_NOBLOCK 0x80
> > > > +
> > >
> > > You can't add a new UAPI flag to fix a kernel-internal problem like this.
> >
> > Sounds fair, see below.
> >
> > What the report indicates is a missing PF_MEMALLOC_NOFS and it's
> > checked on the ashmem side and added as an exception before going
> > to filesystem. On shmem side, no more than a best effort is paid
> > on the inteded exception.
> >
> > --- a/drivers/staging/android/ashmem.c
> > +++ b/drivers/staging/android/ashmem.c
> > @@ -437,6 +437,7 @@ static unsigned long
> > ashmem_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
> > {
> > unsigned long freed = 0;
> > + bool nofs;
> >
> > /* We might recurse into filesystem code, so bail out if necessary */
> > if (!(sc->gfp_mask & __GFP_FS))
> > @@ -445,6 +446,11 @@ ashmem_shrink_scan(struct shrinker *shri
> > if (!mutex_trylock(&ashmem_mutex))
> > return -1;
> >
> > + /* enter filesystem with caution: nonblock on locking */
> > + nofs = current->flags & PF_MEMALLOC_NOFS;
> > + if (!nofs)
> > + current->flags |= PF_MEMALLOC_NOFS;
> > +
> > while (!list_empty(&ashmem_lru_list)) {
> > struct ashmem_range *range =
> > list_first_entry(&ashmem_lru_list, typeof(*range), lru);
>
> I do not think this is an appropriate fix. First of all is this a real
> deadlock or a lockdep false positive? Is it possible that ashmem just
The warning matters and we can do something to quiesce it.
> needs to properly annotate its shmem inodes? Or is it possible that
> the internal backing shmem file is visible to the userspace so the write
> path would be possible?
>
> If this a real problem then the proper fix would be to set internal
> shmem mapping's gfp_mask to drop __GFP_FS.
Thanks for the tip, see below.
Can you expand a bit on how it helps direct reclaimers like khugepaged
in the syzbot report wrt deadlock? TBH I have difficult time following
up after staring at the chart below for quite a while.
Possible unsafe locking scenario:
CPU0CPU1
lock(fs_reclaim);
lock(&sb->s_type->i_mutex_key#15);
lock(fs_reclaim);
lock(&sb->s_type->i_mutex_key#15);
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -381,6 +381,7 @@ static int ashmem_mmap(struct file *file
if (!asma->file) {
char *name = ASHMEM_NAME_DEF;
struct file *vmfile;
+ gfp_t gfp;
if (asma->name[ASHMEM_NAME_PREFIX_LEN] != '\0')
name = asma->name;
@@ -392,6 +393,10 @@ static int ashmem_mmap(struct file *file
goto out;
}
vmfile->f_mode |= FMODE_LSEEK;
+ gfp = mapping_gfp_mask(vmfile->f_mapping);
+ if (gfp & __GFP_FS)
+ mapping_set_gfp_mask(vmfile->f_mapping,
+ gfp & ~__GFP_FS);
asma->file = vmfile;
}
get_file(asma->file);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: possible deadlock in shmem_fallocate (4)
On Tue, 14 Jul 2020 10:32:20 -0700 Suren Baghdasaryan wrote:
> On Tue, Jul 14, 2020 at 9:41 AM Suren Baghdasaryan wrote:
> >
> > On Tue, Jul 14, 2020 at 8:47 AM Todd Kjos wrote:
> > >
> > > +Suren Baghdasaryan +Hridya Valsaraju who support the ashmem driver.
> >
> > Thanks for looping me in.
> >
> > > On Tue, Jul 14, 2020 at 7:18 AM Michal Hocko wrote:
> > > >
> > > > On Tue 14-07-20 22:08:59, Hillf Danton wrote:
> > > > >
> > > > > On Tue, 14 Jul 2020 10:26:29 +0200 Michal Hocko wrote:
> > > > > > On Tue 14-07-20 13:32:05, Hillf Danton wrote:
> > > > > > >
> > > > > > > On Mon, 13 Jul 2020 20:41:11 -0700 Eric Biggers wrote:
> > > > > > > > On Tue, Jul 14, 2020 at 11:32:52AM +0800, Hillf Danton wrote:
> > > > > > > > >
> > > > > > > > > Add FALLOC_FL_NOBLOCK and on the shmem side try to lock inode
> > > > > > > > > upon the
> > > > > > > > > new flag. And the overall upside is to keep the current gfp
> > > > > > > > > either in
> > > > > > > > > the khugepaged context or not.
> > > > > > > > >
> > > > > > > > > --- a/include/uapi/linux/falloc.h
> > > > > > > > > +++ b/include/uapi/linux/falloc.h
> > > > > > > > > @@ -77,4 +77,6 @@
> > > > > > > > > */
> > > > > > > > > #define FALLOC_FL_UNSHARE_RANGE 0x40
> > > > > > > > >
> > > > > > > > > +#define FALLOC_FL_NOBLOCK0x80
> > > > > > > > > +
> > > > > > > >
> > > > > > > > You can't add a new UAPI flag to fix a kernel-internal problem
> > > > > > > > like this.
> > > > > > >
> > > > > > > Sounds fair, see below.
> > > > > > >
> > > > > > > What the report indicates is a missing PF_MEMALLOC_NOFS and it's
> > > > > > > checked on the ashmem side and added as an exception before going
> > > > > > > to filesystem. On shmem side, no more than a best effort is paid
> > > > > > > on the inteded exception.
> > > > > > >
> > > > > > > --- a/drivers/staging/android/ashmem.c
> > > > > > > +++ b/drivers/staging/android/ashmem.c
> > > > > > > @@ -437,6 +437,7 @@ static unsigned long
> > > > > > > ashmem_shrink_scan(struct shrinker *shrink, struct
> > > > > > > shrink_control *sc)
> > > > > > > {
> > > > > > > unsigned long freed = 0;
> > > > > > > + bool nofs;
> > > > > > >
> > > > > > > /* We might recurse into filesystem code, so bail out if
> > > > > > > necessary */
> > > > > > > if (!(sc->gfp_mask & __GFP_FS))
> > > > > > > @@ -445,6 +446,11 @@ ashmem_shrink_scan(struct shrinker *shri
> > > > > > > if (!mutex_trylock(&ashmem_mutex))
> > > > > > > return -1;
> > > > > > >
> > > > > > > + /* enter filesystem with caution: nonblock on locking */
> > > > > > > + nofs = current->flags & PF_MEMALLOC_NOFS;
> > > > > > > + if (!nofs)
> > > > > > > + current->flags |= PF_MEMALLOC_NOFS;
> > > > > > > +
> > > > > > > while (!list_empty(&ashmem_lru_list)) {
> > > > > > > struct ashmem_range *range =
> > > > > > > list_first_entry(&ashmem_lru_list,
> > > > > > > typeof(*range), lru);
> > > > > >
> > > > > > I do not think this is an appropriate fix. First of all is this a
> > > > > > real
> > > > > > deadlock or a lockdep false positive? Is it possible that ashmem
> > > > > > just
> > > > >
> > > > > The warning matters and we can do something to quiesce it.
> > > >
> > > > The underlying issue should be fixed rather than _something_ done to
> > > > silence it.
> > > >
> > > > > > needs to properly annotate its sh
