Re: [PATCH 01/17] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags
From: Christoph Hellwig <hch@infradead.org>
Date: 2017-10-20 07:27:08
Also in:
linux-ext4, linux-fsdevel, linux-xfs, nvdimm
quoted hunk ↗ jump to hunk
if (file) { struct inode *inode = file_inode(file); + unsigned long flags_mask = file->f_op->mmap_supported_flags; + + if (!flags_mask) + flags_mask = LEGACY_MAP_MASK; switch (flags & MAP_TYPE) { case MAP_SHARED: + /* + * Silently ignore unsupported flags - MAP_SHARED has + * traditionally behaved like that and we don't want + * to break compatibility. + */ + flags &= flags_mask; + /* + * Force use of MAP_SHARED_VALIDATE with non-legacy + * flags. E.g. MAP_SYNC is dangerous to use with + * MAP_SHARED as you don't know which consistency model + * you will get. + */ + flags &= LEGACY_MAP_MASK; + /* fall through */ + case MAP_SHARED_VALIDATE: + if (flags & ~flags_mask) + return -EOPNOTSUPP;
Hmmm. I'd expect this to worth more like: case MAP_SHARED: /* Ignore all new flags that need validation: */ flags &= LEGACY_MAP_MASK; /*FALLTHROUGH*/ case MAP_SHARED_VALIDATE: if (flags & ~file->f_op->mmap_supported_flags) return -EOPNOTSUPP; with the legacy mask always implicitly support as indicated in my comment to the XFS patch. Although even the ignoring in MAP_SHARED seems dangerous, but I guess we need that to keep strict backwards compatibility. In world I'd rather do case MAP_SHARED: if (flags & ~LEGACY_MAP_MASK) return -EINVAL;