All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: wangzijie <wangzijie1@honor.com>
Cc: adobriyan@gmail.com, akpm@linux-foundation.org, ast@kernel.org,
	kirill.shutemov@linux.intel.com, linux-fsdevel@vger.kernel.org,
	rick.p.edgecombe@intel.com
Subject: Re: [PATCH] proc: clear FMODE_LSEEK flag correctly for permanent pde
Date: Fri, 6 Jun 2025 04:57:14 +0100	[thread overview]
Message-ID: <20250606035714.GP299672@ZenIV> (raw)
In-Reply-To: <20250606023735.1009957-1-wangzijie1@honor.com>

On Fri, Jun 06, 2025 at 10:37:35AM +0800, wangzijie wrote:

> My bad for making this misbehavior, thank you for helping explain it.
> commit 654b33ada4ab("proc: fix UAF in proc_get_inode()") is in -stable, 
> I refered v1 just for showing race in rmmod scenario, it's my bad.

I still don't understand what's going on.  654b33ada4ab is both in
mainline and in stable, but proc_reg_open() is nowhere near the
shape your patch would imply.

The best reconstruction I can come up with is that an earlier patch
in whatever tree you are talking about has moved

        if (!pde->proc_ops->proc_lseek)
		file->f_mode &= ~FMODE_LSEEK;

down, separately into permanent and non-permanent cases, after use_pde()
in the latter case.  And the author of that earlier patch has moved
the check under if (open) in permanent case, which would warrant that
kind of fixup.

However,
	* why is that earlier patch sitting someplace that is *NOT*
in -next?
	* why bother with those games at all?  Just nick another bit
from pde->flags (let's call it PROC_ENTRY_proc_lseek for consistency
sake), set it in same pde_set_flags() where other flags are dealt with
and just turn that thing into
        if (!pde_has_proc_lseek(pde))
		file->f_mode &= ~FMODE_LSEEK;
leaving it where it is.  Less intrusive and easier to follow...

	Call it something like

check proc_lseek needs the same treatment as ones for proc_read_iter et.al.

and describe it as a gap in "proc: fix UAF in proc_get_inode()",
fixed in exact same manner...

  reply	other threads:[~2025-06-06  3:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-05  6:52 [PATCH] proc: clear FMODE_LSEEK flag correctly for permanent pde wangzijie
2025-06-05 21:44 ` Andrew Morton
2025-06-06  1:56   ` Al Viro
2025-06-06  2:37     ` wangzijie
2025-06-06  3:57       ` Al Viro [this message]
2025-06-06  4:01         ` Andrew Morton
2025-06-06  5:13         ` wangzijie
2025-06-06  3:34     ` wangzijie
2025-06-06  3:44     ` Andrew Morton
2025-06-06  3:58       ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250606035714.GP299672@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=ast@kernel.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=wangzijie1@honor.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.