Re: Fwd: BadBunny: UFFDIO_COPY shmem killpriv bypass leading to local privilege escalation

On Mon 17-08-26 16:18:16, Matthew Wilcox wrote:
> On Mon, Aug 17, 2026 at 04:53:08PM +0200, Jan Kara wrote:
> > On Tue 11-08-26 14:16:31, vova tokarev wrote:
> > > Matthew, Christian, Pedro,
> > > 
> > > Lol - fair point, I'll keep the reggaeton references out of future
> > > commit messages.
> > > 
> > > Agreed on severity - but this is a
> > > killpriv invariant violation, and the kernel has treated those as
> > > security fixes worth backporting before.
> > 
> > I tend to agree this would be good to fix for consistency as much as I also
> > don't think this is a particularly serious issue. Removing SUID bits on
> > write is more of a hardening measure than serious security guarantee and is
> > there mostly for historical reasons AFAIK from the old Unix days.  POSIX
> > for write(2) mentions this as: "Upon successful completion, where nbyte is
> > greater than 0, write() shall mark for update the st_ctime and st_mtime
> > fields of the file, and if the file is a regular file, the S_ISUID and
> > S_ISGID bits of the file mode may be cleared." truncate(), chmod(), chown()
> > have similar notes. No mention of this when handling mmap BTW :).
> > 
> > Anyway, the problem with calling __file_remove_privs() when writing through
> > mmap is with the implementation - in particular the locking. To be able to
> > call __file_remove_privs() we need to hold i_rwsem and that ranks above VMA
> > locks / mmap_lock we hold during the page fault. Now that I'm speaking
> > about it I even have a vague recollection this has already come up in the
> > past and we've just decided to leave it alone due to these technical
> > difficulties.
> 
> Perhaps we could just do it at mmap() time rather than waiting for the
> first fault?  Obviously only for MAP_SHARED / PROT_WRITE.  I could see
> an unenlightened interpreter doing unnecessary MAP_SHARED / PROT_WRITE
> mappings, so that might not fly.

Yes, I'd be afraid of userspace regressions here.

> We could also do it within filemap_fault() (and shmem_fault()).  If the
> fault is a write and the suid bit is set, drop the locks, clear the suid
> bit and return VM_FAULT_RETRY.  We already do that if we need to do
> readahead, so it's a well-tested path.

Yeah, right, dropping the locks + VM_FAULT_RETRY would do it given how
rare case that should be. Except we can have faults without
FAULT_FLAG_ALLOW_RETRY (generally from GUP callers) for which we cannot do
this... :-|

								Honza
-- 
Jan Kara <[email protected]>
SUSE Labs, CR