Re: [git pull] drm fixes

On Mon, Jun 07, 2010 at 08:32:45PM +0100, David Woodhouse wrote: The fix is fairly trivial. There's a big patch to fs/jffs2/dir.c which accounts for the bulk of my pull request, but if you look harder you'll see it's mostly just a bunch of removing 'return ret;' and adding 'goto fail;' so the

Re: [git pull] drm fixes

On Mon, Jun 07, 2010 at 10:39:28PM +0100, David Woodhouse wrote: On Mon, 2010-06-07 at 14:17 -0700, Linus Torvalds wrote: and that changelog doesn't really explain it either (fix leak? Ok, I can see the iput() fixing the leak - but you also did that jffs2_clear_inode() change, and that

[RFC] deadlock in drm/exynos: fix wrong pointer access at vm close

You have drm_dev-struct_mutex grabbed before -mmap_sem in exynos_drm_gem_mmap_ioctl() and after - in exynos_drm_gem_fault() (since -fault() is always called with -mmap_sem held). Looks like a garden-variety AB-BA deadlock... Incidentally, what should happen if another process

Re: [RFC] deadlock in drm/exynos: fix wrong pointer access at vm close

On Mon, Sep 23, 2013 at 04:49:30PM +0900, Inki Dae wrote: Hi, -Original Message- From: Al Viro [mailto:v...@ftp.linux.org.uk] On Behalf Of Al Viro Sent: Monday, September 23, 2013 6:29 AM To: YoungJun Cho Cc: dri-devel@lists.freedesktop.org; Inki Dae Subject: [RFC] deadlock

Re: [RFC] deadlock in drm/exynos: fix wrong pointer access at vm close

On Tue, Sep 24, 2013 at 01:41:00PM +0900, Inki Dae wrote: I can't see to hold -mmap_sem when it calls find_vma() anywhere else. Er... What, in your opinion, would protect the result of find_vma(), if not that? E.g. if another thread does munmap() on that area... vma isn't refcounted; there

Re: [RFC] deadlock in drm/exynos: fix wrong pointer access at vm close

On Wed, Sep 25, 2013 at 01:34:30PM +0900, Inki Dae wrote: It seems that we can use a new anon file instead of using drm file to resolve the issue. Could you describe what are you trying to achieve with that ioctl() and what semantics do you want from normal mmap()?

Re: [PATCH v2 1/2] anon_inodes: allow external inode allocations

. As fs/anon_inodes.c already provides a minimal internal FS mount, we extend it to also provide anonymous inodes for use in drivers like DRM. Signed-off-by: David Herrmann dh.herrm...@gmail.com Wanted-by: Daniel Vetter daniel.vet...@ffwll.ch Told-to-stop-abusing-anon_inodes-by: Al Viro v

[RFC 0/6] Module for tracking/accounting shared memory buffers

On Tue, Oct 11, 2016 at 04:50:04PM -0700, Ruchi Kandoi wrote: > memtrack maintains a per-process list of shared buffer references, which is > exported to userspace as /proc/[pid]/memtrack. Buffers can be optionally > "tagged" with a short string: for example, Android userspace would use this >

Re: [PATCH RESEND] drm/via: use get_user_pages_unlocked()

On Tue, Feb 28, 2017 at 10:01:10AM +0100, Daniel Vetter wrote: > > + ret = get_user_pages_unlocked((unsigned long)xfer->mem_addr, > > + vsg->num_pages, vsg->pages, > > + (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0); Umm... Why not ret =

Use of copy_from_user in msm_gem_submit.c while holding a spin_lock

On Wed, Aug 17, 2016 at 02:49:32PM -0400, Rob Clark wrote: > I'm not saying that I shouldn't fix it (although not quite sure how > yet.. taking/dropping the spinlock inside the loop is not a good > option from a performance standpoint). What I am saying is that this > is not something that can

Use of copy_from_user in msm_gem_submit.c while holding a spin_lock

On Wed, Aug 17, 2016 at 03:24:38PM -0400, Rob Clark wrote: > hmm, looks like, at least on arm (not sure about arm64), > > #define __copy_from_user_inatomic __copy_from_user > > ie. copy_from_user() minus the access_ok() and memset in the > !access_ok() path.. but maybe what I want is just the >

Use of copy_from_user in msm_gem_submit.c while holding a spin_lock

On Wed, Aug 17, 2016 at 11:08:46AM -0400, Rob Clark wrote: > On Wed, Aug 17, 2016 at 7:40 AM, Vaishali Thakkar > wrote: > > Hello, > > > > I was wondering about the call to copy_from_user in function > > submit_lookup_objects for drive > > /gpu/drm/msm/msm_gem_submit.c It calls

[PATCH] fs: export simple_dname for drm drivers.

On Mon, Jan 20, 2014 at 10:54:44AM +1000, Dave Airlie wrote: > David Herrmann's changes to use a pseudo filesystem for drm's shared > inodes requires this be exported for drm to build as a module. > > I'd like to merge this via the drm tree, so please ack. Having looked through these patches...

[PATCH] hfsplus: Remove hfsplus_file_lookup

On Wed, Dec 11, 2013 at 10:49:29PM +0300, Vyacheslav Dubeyko wrote: > This feature worked earlier under Linux. So, I suppose that some changes in > HFS+ driver > or in VFS broke it. And it needs to investigate and fix the reported issue. > Thank you for the > report. This "feature" is severely

[git pull] drm fixes

On Mon, Jun 07, 2010 at 11:53:28AM -0700, Linus Torvalds wrote: > > > On Mon, 7 Jun 2010, Al Viro wrote: > > > > Ho-hum... Speaking of which, what about leak fixes? There's a > > long-standing > > in-core inode leak in jffs2; basically, if you fail dire

[git pull] drm fixes

On Mon, Jun 07, 2010 at 11:00:51AM -0700, Linus Torvalds wrote: > So please. Just make me a tree that has regression fixes _only_. I'm not > AT ALL interested in "it is useful to report the gpu temp". If it was so > useful, and if it was ready before the merge window, it should hav gone in >

[git pull] drm fixes

On Mon, Jun 07, 2010 at 08:32:45PM +0100, David Woodhouse wrote: > The fix is fairly trivial. There's a "big" patch to fs/jffs2/dir.c which > accounts for the bulk of my pull request, but if you look harder you'll > see it's mostly just a bunch of removing 'return ret;' and adding > 'goto fail;'

[git pull] drm fixes

On Mon, Jun 07, 2010 at 02:17:23PM -0700, Linus Torvalds wrote: > jffs2_clear_inode(inode); > > into > > make_bad_inode(inode); > iput(inode); > > and that changelog doesn't really explain it either ("fix leak"? Ok, I can > see the iput() fixing the leak - but you also did

[git pull] drm fixes

On Mon, Jun 07, 2010 at 10:39:28PM +0100, David Woodhouse wrote: > On Mon, 2010-06-07 at 14:17 -0700, Linus Torvalds wrote: > > and that changelog doesn't really explain it either ("fix leak"? Ok, I can > > see the iput() fixing the leak - but you also did that jffs2_clear_inode() > > change,

Re: [PATCH] drm: Add missing field copy in compat_drm_version

an you pls apply this directly? It's a fumble > in Al's rework. Direct mbox link from patchwork, in case you don't have > that in your archives anywhere: (Belated) ACKed-by: Al Viro <v...@zeniv.linux.org.uk> ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH][drm-next] drm compat: ensure mode in drm_agp_info is being copied

On Tue, Jul 04, 2017 at 05:48:21PM +0100, Colin King wrote: > From: Colin Ian King > > A recent compat change removed the copying of i32.mode from info.mode. > Add it back in to fix this removal as we currently are leaking information > from the stack. > > Detected by

Re: __user with scalar data types

On Mon, Jun 19, 2017 at 10:32:18PM +0200, Luc Van Oostenryck wrote: > On Mon, Jun 19, 2017 at 10:15:09AM -0600, Jordan Crouse wrote: > > struct uapistruct { > > ... > > __u64 __user myptr; > > --- > > }; > > > > And then converting it for use in the kernel as such: > > > > { > >

Re: __user with scalar data types

On Mon, Jun 19, 2017 at 10:15:09AM -0600, Jordan Crouse wrote: > Which raised a bikeshed debate over whether it is appropriate to mark a scalar > type as __user. My opinion is that it is appropriate because __user should > mark > user memory regardless of the container. What the hell? __user

Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg

On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > Acked-by: Darren Hart (VMware) > > As for a longer term solution, would it be possible to init fops in such > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > so we don't have to duplicate this boilerplate

Re: INFO: rcu detected stall in sys_sendfile64 (2)

On Mon, Mar 11, 2019 at 08:59:00PM -0700, syzbot wrote: > syzbot has bisected this bug to: > > commit 34e07e42c55aeaa78e93b057a6664e2ecde3fadb > Author: Chris Wilson > Date: Thu Feb 8 10:54:48 2018 + > > drm/i915: Add missing kerneldoc for 'ent' in i915_driver_init_early > >

Re: [PATCH v3 14/26] compat_ioctl: move ATYFB_CLK handling to atyfb driver

On Tue, Apr 16, 2019 at 10:25:35PM +0200, Arnd Bergmann wrote: > +static int atyfb_compat_ioctl(struct fb_info *info, u_int cmd, u_long arg) > +{ > + return atyfb_ioctl(info, cmd, (u_long)compat_ptr(arg)); > +} > +#endif Huh? Why isn't that using compat_ioctl_ptr()?

Re: [PATCH v3 14/26] compat_ioctl: move ATYFB_CLK handling to atyfb driver

On Wed, Apr 17, 2019 at 10:27:00PM +0100, Al Viro wrote: > On Tue, Apr 16, 2019 at 10:25:35PM +0200, Arnd Bergmann wrote: > > +static int atyfb_compat_ioctl(struct fb_info *info, u_int cmd, u_long arg) > > +{ > > + return atyfb_ioctl(info, cmd, (u_long)compat_ptr(arg)

Re: [PATCH v2] drm: return -EFAULT if copy_to_user() fails

gt; the callers treat it as a successful copy. > > > > Signed-off-by: Dan Carpenter > > Thanks Dan, I've applied this to drm-misc-fixes. FWIW, Acked-by: Al Viro

Re: [PATCH] drm: return -EFAULT if copy_one_buf() fails

On Tue, Jun 18, 2019 at 03:56:23PM +0300, Dan Carpenter wrote: > The copy_to_user() function returns the number of bytes remaining to be > copied, but we want to return -EFAULT. This function is called from > __drm_legacy_infobufs() which expects negative error codes. > > Fixes: 5c7640ab6258

Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl

On Thu, Apr 25, 2019 at 12:21:53PM -0300, Mauro Carvalho Chehab wrote: > If I understand your patch description well, using compat_ptr_ioctl > only works if the driver is not for s390, right? No; s390 is where "oh, just set ->compat_ioctl same as ->unlocked_ioctl and be done with that;

Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl

On Thu, Apr 25, 2019 at 05:55:23PM +0200, Arnd Bergmann wrote: > On Thu, Apr 25, 2019 at 5:35 PM Al Viro wrote: > > > > On Thu, Apr 25, 2019 at 12:21:53PM -0300, Mauro Carvalho Chehab wrote: > > > > > If I understand your patch description well, using com

Re: [PATCHv2 2/3] i915: convert to new mount API

On Tue, Aug 06, 2019 at 12:50:10AM -0700, Hugh Dickins wrote: > that mapping must have been decided previously). In Google we do use > fcntls F_HUGEPAGE and F_NOHUGEPAGE to override on a per-file basis - > one day I'll get to upstreaming those. That'd be nice - we could kill the i915 wierd

Re: [PATCHv2 2/3] i915: convert to new mount API

On Wed, Aug 07, 2019 at 08:30:02AM +0200, Christoph Hellwig wrote: > On Tue, Aug 06, 2019 at 12:50:10AM -0700, Hugh Dickins wrote: > > Though personally I'm averse to managing "f"objects through > > "m"interfaces, which can get ridiculous (notably, MADV_HUGEPAGE works > > on the virtual address of

Re: [PATCHv2 2/3] i915: convert to new mount API

On Mon, Aug 05, 2019 at 07:12:55PM +0100, Al Viro wrote: > On Tue, Aug 06, 2019 at 01:03:06AM +0900, Sergey Senozhatsky wrote: > > tmpfs does not set ->remount_fs() anymore and its users need > > to be converted to new mount API. > > Could you explain why the devil do

Re: [PATCHv2 2/3] i915: convert to new mount API

On Tue, Aug 06, 2019 at 01:03:06AM +0900, Sergey Senozhatsky wrote: > tmpfs does not set ->remount_fs() anymore and its users need > to be converted to new mount API. Could you explain why the devil do you bother with remount at all? Why not pass the right options when mounting the damn thing?

Re: [PATCHv2 2/3] i915: convert to new mount API

On Mon, Aug 05, 2019 at 07:12:55PM +0100, Al Viro wrote: > On Tue, Aug 06, 2019 at 01:03:06AM +0900, Sergey Senozhatsky wrote: > > tmpfs does not set ->remount_fs() anymore and its users need > > to be converted to new mount API. > > Could you explain why the devil do

Re: [PATCH V1 09/10] arch/kmap: Define kmap_atomic_prot() for all arch's

On Fri, May 01, 2020 at 03:37:34AM +0100, Al Viro wrote: > On Thu, Apr 30, 2020 at 01:38:44PM -0700, ira.we...@intel.com wrote: > > > -static inline void *kmap_atomic(struct page *page) > > +static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot) > > {

Re: [PATCH V1 05/10] arch/kmap_atomic: Consolidate duplicate code

On Thu, Apr 30, 2020 at 01:38:40PM -0700, ira.we...@intel.com wrote: > From: Ira Weiny > > Every arch has the same code to ensure atomic operations and a check for > !HIGHMEM page. > > Remove the duplicate code by defining a core kmap_atomic() which only > calls the arch specific

Re: [PATCH V1 09/10] arch/kmap: Define kmap_atomic_prot() for all arch's

On Thu, Apr 30, 2020 at 01:38:44PM -0700, ira.we...@intel.com wrote: > -static inline void *kmap_atomic(struct page *page) > +static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot) > { > preempt_disable(); > pagefault_disable(); > if (!PageHighMem(page)) >

Re: [PATCH V1 08/10] arch/kmap: Don't hard code kmap_prot values

On Thu, Apr 30, 2020 at 01:38:43PM -0700, ira.we...@intel.com wrote: > From: Ira Weiny > > To support kmap_atomic_prot() on all architectures each arch must > support protections passed in to them. > > Change csky, mips, nds32 and xtensa to use their global kmap_prot value > rather than a hard

Re: [PATCH V2 00/11] Subject: Remove duplicated kmap code

On Sun, May 03, 2020 at 10:04:47PM -0700, Ira Weiny wrote: > Grepping for 'asm/highmem.h' and investigations don't reveal any issues... > But > you do have me worried. That said 0-day has been crunching on multiple > versions of this series without issues such as this (save the mips issue >

Re: [PATCH V2 00/11] Subject: Remove duplicated kmap code

On Sun, May 03, 2020 at 06:09:01PM -0700, ira.we...@intel.com wrote: > From: Ira Weiny > > The kmap infrastructure has been copied almost verbatim to every architecture. > This series consolidates obvious duplicated code by defining core functions > which call into the architectures only when

Re: [PATCH V2 00/11] Subject: Remove duplicated kmap code

On Mon, May 04, 2020 at 01:17:41PM -0700, Ira Weiny wrote: > > || * arm: much, much worse. We have several files that pull > > linux/highmem.h: > > || arch/arm/mm/cache-feroceon-l2.c, arch/arm/mm/cache-xsc3l2.c, > > || arch/arm/mm/copypage-*.c, arch/arm/mm/dma-mapping.c, arch/arm/mm/flush.c, >

Re: [PATCH] arch/{mips,sparc,microblaze,powerpc}: Don't enable pagefault/preempt twice

On Thu, May 21, 2020 at 03:20:46PM -0700, Guenter Roeck wrote: > On 5/21/20 10:27 AM, Al Viro wrote: > > On Tue, May 19, 2020 at 09:54:22AM -0700, Guenter Roeck wrote: > >> On Mon, May 18, 2020 at 11:48:43AM -0700, ira.we...@intel.com wrote: > >>> From: Ira Weiny

Re: [PATCH] arch/{mips,sparc,microblaze,powerpc}: Don't enable pagefault/preempt twice

On Thu, May 21, 2020 at 06:11:08PM -0700, Guenter Roeck wrote: > Mainline, with: > > qemu-system-sparc -M SS-4 -kernel arch/sparc/boot/zImage -no-reboot \ > -snapshot -drive file=rootfs.ext2,format=raw,if=scsi \ > -append "panic=-1 slub_debug=FZPUA root=/dev/sda console=ttyS0" >

Re: [PATCH] arch/{mips,sparc,microblaze,powerpc}: Don't enable pagefault/preempt twice

On Tue, May 19, 2020 at 09:54:22AM -0700, Guenter Roeck wrote: > On Mon, May 18, 2020 at 11:48:43AM -0700, ira.we...@intel.com wrote: > > From: Ira Weiny > > > > The kunmap_atomic clean up failed to remove one set of pagefault/preempt > > enables when vaddr is not in the fixmap. > > > > Fixes:

Re: [PATCH] arch/{mips,sparc,microblaze,powerpc}: Don't enable pagefault/preempt twice

On Thu, May 21, 2020 at 11:46:12PM +0100, Al Viro wrote: > On Thu, May 21, 2020 at 03:20:46PM -0700, Guenter Roeck wrote: > > On 5/21/20 10:27 AM, Al Viro wrote: > > > On Tue, May 19, 2020 at 09:54:22AM -0700, Guenter Roeck wrote: > > >> On Mon, May 18, 20

Re: [PATCH] arch/{mips,sparc,microblaze,powerpc}: Don't enable pagefault/preempt twice

On Fri, May 22, 2020 at 02:29:50AM +0100, Al Viro wrote: > On Thu, May 21, 2020 at 06:11:08PM -0700, Guenter Roeck wrote: > > > Mainline, with: > > > > qemu-system-sparc -M SS-4 -kernel arch/sparc/boot/zImage -no-reboot \ > > -snapshot -drive file=

Re: [PATCH RFC PKS/PMEM 33/58] fs/cramfs: Utilize new kmap_thread()

On Tue, Oct 13, 2020 at 08:36:43PM +0100, Matthew Wilcox wrote: > static inline void copy_to_highpage(struct page *to, void *vfrom, unsigned > int size) > { > char *vto = kmap_atomic(to); > > memcpy(vto, vfrom, size); > kunmap_atomic(vto); > } > > in linux/highmem.h ? You

Re: [PATCH 2/2] dmabuf/tracing: Add dma-buf trace events

On Mon, Aug 03, 2020 at 11:28:31PM +0100, Al Viro wrote: > IOW, what the hell is that horror for? You do realize, for example, that > there's > such thing as dup(), right? And dup2() as well. And while we are at it, how > do you keep track of removals, considering the fact that y

Re: [PATCH 2/2] dmabuf/tracing: Add dma-buf trace events

On Mon, Aug 03, 2020 at 09:22:53AM -0700, Suren Baghdasaryan wrote: > On Mon, Aug 3, 2020 at 9:12 AM Matthew Wilcox wrote: > > > > On Mon, Aug 03, 2020 at 09:00:00AM -0700, Suren Baghdasaryan wrote: > > > On Mon, Aug 3, 2020 at 8:41 AM Matthew Wilcox wrote: > > > > > > > > On Mon, Aug 03, 2020

Re: [PATCH 1/2] fs: Add fd_install file operation

On Mon, Aug 03, 2020 at 02:47:18PM +, Kalesh Singh wrote: > Provides a per process hook for the acquisition of file descriptors, > despite the method used to obtain the descriptor. No, with the side of Fuck, No. Driver has no possible reason to watch know the descriptors involved. Moreover,

Re: [PATCH 2/2] dmabuf/tracing: Add dma-buf trace events

On Tue, Aug 04, 2020 at 03:44:51PM +, Kalesh Singh wrote: > Hi Al. Thank you for the comments. Ultimately what we need is to identify > processes > that hold a file reference to the dma-buf. Unfortunately we can't use only > explicit dma_buf_get/dma_buf_put to track them because when an FD

Re: [PATCH v2 1/2] video: fbdev: amifb: add FIXME about dead APUS support

ded a PPC board. APUS support was killed off a long time ago, > > > when arch/ppc/ was still king, but these #ifdefs were missed, because > > > they didn't test for CONFIG_APUS. > > > > Add FIXME about using the C code variants (APUS ones) in the future. >

Re: [PATCH 4/9] drm: remove the drm file system

On Tue, Mar 09, 2021 at 04:53:43PM +0100, Christoph Hellwig wrote: > Just use the generic anon_inode file system. Are you changing the lifetime rules for that module? ___ dri-devel mailing list dri-devel@lists.freedesktop.org

Re: [PATCH 3/9] powerpc/pseries: remove the ppc-cmm file system

On Tue, Mar 09, 2021 at 04:53:42PM +0100, Christoph Hellwig wrote: > Just use the generic anon_inode file system. Umm... The only problem I see here is the lifetime rules for that module, and that's not something introduced in this patchset. Said that, looks like the logics around that place is

Re: [PATCH v2] fs: clean up usage of noop_dirty_folio

re theory; sure, missing that would've probably ended up with broken build, but that can easily be dependent upon the config (and that, alas, is also not a pure theory - BTDT). In this case the change is correct, fortunately... Other than that part of commit message - Acked-by: Al Viro

Re: [PATCH 5/6] dma-buf: remove useless FMODE_LSEEK flag

On Fri, Jun 24, 2022 at 06:56:30PM +0200, Jason A. Donenfeld wrote: > This is already on by default. ITYM "anon_inode_getfile() has already set it, since dma_buf_fops has non-NULL ->llseek".

[PATCH 6/8] dma_buf: no need to bother with file_inode()->i_mapping

->f_mapping will do just fine Signed-off-by: Al Viro --- drivers/dma-buf/udmabuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 38e8767ec371..210473d927d8 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/

Re: [PATCH] dma-buf/sync_file: Use fdget()

On Fri, May 05, 2023 at 11:03:39AM +0800, ye.xingc...@zte.com.cn wrote: > From: Ye Xingchen > > convert the fget() use to fdget(). NAK.

Re: [PATCH] dma-buf: Use fdget()

On Fri, May 05, 2023 at 10:18:47AM +0800, ye.xingc...@zte.com.cn wrote: > From: Ye Xingchen > > convert the fget() use to fdget(). NAK.

Re: [PATCH] dma-buf/sync_file: Use fdget()

On Fri, May 05, 2023 at 10:22:09AM +0200, Christian König wrote: > Am 05.05.23 um 05:03 schrieb ye.xingc...@zte.com.cn: > > From: Ye Xingchen > > > > convert the fget() use to fdget(). > > Well the rational is missing. Why should we do that? We very definitely should not. The series appears

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Fri, May 03, 2024 at 02:52:38PM -0700, Linus Torvalds wrote: > On Fri, 3 May 2024 at 14:45, Al Viro wrote: > > > > How do you get through eventpoll_release_file() while someone > > has entered ep_item_poll()? > > Doesn't matter. > > Look, it's enough that the

Re: get_file() unsafe under epoll (was Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove)

On Fri, May 03, 2024 at 01:28:37PM -0700, Kees Cook wrote: > > Is this the right approach? It still feels to me like get_file() needs > to happen much earlier... I don't believe it needs to happen at all. The problem is not that ->release() can be called during ->poll() - it can't and it

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Fri, May 03, 2024 at 03:46:25PM -0700, Kees Cook wrote: > On Fri, May 03, 2024 at 02:52:38PM -0700, Linus Torvalds wrote: > > That means that the file will be released - and it means that you have > > violated all the refcounting rules for poll(). > > I feel like I've been looking at this too

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Fri, May 03, 2024 at 04:16:15PM -0700, Linus Torvalds wrote: > On Fri, 3 May 2024 at 15:07, Al Viro wrote: > > > > Suppose your program calls select() on a pipe and dmabuf, sees data to be > > read > > from pipe, reads it, closes both pipe and dmabuf and exit

Re: get_file() unsafe under epoll (was Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove)

On Fri, May 03, 2024 at 02:42:22PM -0700, Linus Torvalds wrote: > On Fri, 3 May 2024 at 14:36, Al Viro wrote: > > > > ... the last part is no-go - poll_wait() must be able to grab a reference > > (well, the callback in it must) > > Yeah. I really think that *poll*

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Fri, May 03, 2024 at 11:01:45PM +0100, Al Viro wrote: > Having ->poll() instance itself grab reference is really asking for problem, > even on the boxen that have CONFIG_EPOLL turned off. select(2) is enough; > it will take care of references grabbed by __pollwait(), but it do

Re: get_file() unsafe under epoll (was Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove)

On Fri, May 03, 2024 at 02:24:45PM -0700, Linus Torvalds wrote: > Because even with perfectly normal "->poll()", and even with the > ep_item_poll() happening *before* eventpoll_release_file(), you have > this trivial race: > > ep_item_poll() > ->poll() > > and *between* those two

Re: get_file() unsafe under epoll (was Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove)

On Fri, May 03, 2024 at 10:11:09PM +0100, Al Viro wrote: > On Fri, May 03, 2024 at 01:28:37PM -0700, Kees Cook wrote: > > > > Is this the right approach? It still feels to me like get_file() needs > > to happen much earlier... > > I don't believe it needs to

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Fri, May 03, 2024 at 02:33:37PM -0700, Linus Torvalds wrote: > Look at the hack in __ep_remove(): if it is concurrent with > eventpoll_release_file(), it will hit this code > > spin_lock(>f_lock); > if (epi->dying && !force) { > spin_unlock(>f_lock); >

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Fri, May 03, 2024 at 02:11:30PM -0700, Linus Torvalds wrote: > epoll is a mess, and does various invalid things in the name of > performance. > > Let's try to rein it in a bit. Something like this, perhaps? > +/* > + * The ffd.file pointer may be in the process of > + * being torn down due to

Re: [PATCH 5/5] fs: Convert struct file::f_count to refcount_long_t

On Thu, May 02, 2024 at 05:10:18PM -0700, Kees Cook wrote: > But anyway, there needs to be a general "oops I hit 0"-aware form of > get_file(), and it seems like it should just be get_file() itself... ... which brings back the question of what's the sane damage mitigation for that. Adding

Re: [PATCH 5/5] fs: Convert struct file::f_count to refcount_long_t

count_long_t. It is used on fairly hot paths. What's more, it's not at all obvious what the hell would right semantics be. NAKed-by: Al Viro

Re: [PATCH 5/5] fs: Convert struct file::f_count to refcount_long_t

On Thu, May 02, 2024 at 03:52:21PM -0700, Kees Cook wrote: > As for semantics, what do you mean? Detecting dec-below-zero means we > catch underflow, and detected inc-from-zero means we catch resurrection > attempts. In both cases we avoid double-free, but we have already lost > to a potential

Re: [PATCH 5/5] fs: Convert struct file::f_count to refcount_long_t

On Thu, May 02, 2024 at 04:21:13PM -0700, Kees Cook wrote: > On Fri, May 03, 2024 at 12:12:28AM +0100, Al Viro wrote: > > On Thu, May 02, 2024 at 03:52:21PM -0700, Kees Cook wrote: > > > > > As for semantics, what do you mean? Detecting dec-below-zero means we > > &

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Sat, May 04, 2024 at 08:53:47AM -0700, Linus Torvalds wrote: > poll_wait > -> __pollwait > -> get_file (*boom*) > > but the boom is very small because the poll_wait() will be undone by > poll_freewait(), and normally poll/select has held the file count > elevated. Not quite. It's

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Sun, May 05, 2024 at 09:46:05AM -0700, Linus Torvalds wrote: > WHY? > > Why cannot you and Al just admit that the problem is in epoll. Always > has been, always will be. Nobody (well, nobody who'd ever read epoll) argues that epoll is not a problem. > The fact is, it's not dma-buf that is

Re: [PATCH] epoll: try to be a _bit_ better about file lifetimes

On Sun, May 05, 2024 at 01:03:07PM -0700, Linus Torvalds wrote: > On Sun, 5 May 2024 at 12:46, Al Viro wrote: > > > > I've no problem with having epoll grab a reference, but if we make that > > a universal requirement ->poll() instances can rely upon, >