linux-kernel - Re: [PATCH v2] ipc: Use private shmem or hugetlbfs inodes for shm segments.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LSU.2.11.1507271411270.2122@eggly.anvils>
Date:	Mon, 27 Jul 2015 14:12:54 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	Stephen Smalley <sds@...ho.nsa.gov>
cc:	Hugh Dickins <hughd@...gle.com>, prarit@...hat.com,
	david@...morbit.com, mstevens@...oraproject.org,
	manfred@...orfullife.com, esandeen@...hat.com, wagi@...om.org,
	linux-kernel@...r.kernel.org, eparis@...hat.com,
	linux-mm@...ck.org, linux-security-module@...r.kernel.org,
	dave@...olabs.net, nyc@...omorphy.com, akpm@...ux-foundation.org,
	torvalds@...ux-foundation.org, selinux@...ho.nsa.gov
Subject: Re: [PATCH v2] ipc: Use private shmem or hugetlbfs inodes for shm
 segments.

On Mon, 27 Jul 2015, Stephen Smalley wrote:
> On 07/27/2015 03:32 PM, Hugh Dickins wrote:
> > On Fri, 24 Jul 2015, Stephen Smalley wrote:
> >> --- a/fs/hugetlbfs/inode.c
> >> +++ b/fs/hugetlbfs/inode.c
> >> @@ -1010,6 +1010,8 @@ struct file *hugetlb_file_setup(const char *name, size_t size,
> >>  	inode = hugetlbfs_get_inode(sb, NULL, S_IFREG | S_IRWXUGO, 0);
> >>  	if (!inode)
> >>  		goto out_dentry;
> >> +	if (creat_flags == HUGETLB_SHMFS_INODE)
> >> +		inode->i_flags |= S_PRIVATE;
> > 
> > I wonder if you would do better just to set S_PRIVATE unconditionally
> > there.
> > 
> > hugetlb_file_setup() has two callsites, neither of which exposes an fd.
> > One of them is shm.c's newseg(), which is getting us into the lockdep
> > trouble that you're fixing here.
> > 
> > The other is mmap.c's mmap_pgoff().  Now I don't think that will ever
> > get into lockdep trouble (no mutex or rwsem has been taken at that
> > point), but might your change above introduce (perhaps now or perhaps
> > in future) an inconsistency between how SElinux checks are applied to
> > a SHM area, and how they are applied to a MAP_ANONYMOUS|MAP_HUGETLB
> > area, and how they are applied to a straight MAP_ANONYMOUS area?
> > 
> > I think your patch as it stands brings SHM into line with
> > MAP_ANONYMOUS, but leaves MAP_ANONYMOUS|MAP_HUGETLB going the old way.
> > Perhaps an anomaly would appear when mprotect() is used?
> > 
> > It's up to you: I think your patch is okay as is,
> > but I just wonder if it has a surprise in store for the future.
> 
> That sounds reasonable, although there is the concern that
> hugetlb_file_setup() might be used in the future for files that are
> exposed as fds, unless we rename it to hugetlb_kernel_file_setup() or

Good idea.

> similar to match shmem_kernel_file_setup().  Also should probably be
> done as a separate change on top since it isn't directly related to
> ipc/shm or fixing this lockdep.

Fair enough.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/