Messages in this thread | | | Date | Thu, 15 Oct 2009 10:21:49 +0800 | Subject | Re: [PATCH] mm/nommu.c: Fix improperly call of security API in mmap | From | graff yang <> |
| |
Hi, David, Thanks your patch, I will test it.
On Wed, Oct 14, 2009 at 10:08 PM, David Howells <dhowells@redhat.com> wrote: > <graff.yang@gmail.com> wrote: > >> The original code calling security_file_mmap() use user's hint address >> as it's 5th argument(addr). This is improper, as the hint address may be >> NULL. >> In this case, the security_file_mmap() may incorrectly return -EPERM. >> >> This patch moved the calling of security_file_mmap() out of >> validate_mmap_request() to do_mmap_pgoff(), and call this >> security API with the address that attempting to mmap. > > I think this is the wrong approach. Firstly, the hint is cleared in NOMMU > mode, and secondly, I think that the check against the minimum LSM address is > pointless in NOMMU mode too. > > So I think the attached patch is a better approach. > > David > --- > From: David Howells <dhowells@redhat.com> > > NOMMU: Ignore the address parameter in the file_mmap() security check > > Ignore the address parameter in the various file_mmap() security checks when > CONFIG_MMU=n as the address hint is ignored under those circumstances, and in > any case the minimum mapping address check is pointless in NOMMU mode. > > Signed-off-by: David Howells <dhowells@redhat.com> > --- > > include/linux/security.h | 1 + > mm/nommu.c | 2 +- > security/commoncap.c | 2 ++ > security/selinux/hooks.c | 2 ++ > 4 files changed, 6 insertions(+), 1 deletions(-) > > > diff --git a/include/linux/security.h b/include/linux/security.h > index 239e40d..0583f16 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -593,6 +593,7 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) > * @reqprot contains the protection requested by the application. > * @prot contains the protection that will be applied by the kernel. > * @flags contains the operational flags. > + * @addr contains the mapping address, and should be ignored in NOMMU mode. > * Return 0 if permission is granted. > * @file_mprotect: > * Check permissions before changing memory access permissions. > diff --git a/mm/nommu.c b/mm/nommu.c > index 3c3b4b2..cfea46c 100644 > --- a/mm/nommu.c > +++ b/mm/nommu.c > @@ -974,7 +974,7 @@ static int validate_mmap_request(struct file *file, > } > > /* allow the security API to have its say */ > - ret = security_file_mmap(file, reqprot, prot, flags, addr, 0); > + ret = security_file_mmap(file, reqprot, prot, flags, 0, 0); > if (ret < 0) > return ret; > > diff --git a/security/commoncap.c b/security/commoncap.c > index fe30751..ac1f745 100644 > --- a/security/commoncap.c > +++ b/security/commoncap.c > @@ -1005,6 +1005,7 @@ int cap_file_mmap(struct file *file, unsigned long reqprot, > { > int ret = 0; > > +#ifdef CONFIG_MMU > if (addr < dac_mmap_min_addr) { > ret = cap_capable(current, current_cred(), CAP_SYS_RAWIO, > SECURITY_CAP_AUDIT); > @@ -1012,5 +1013,6 @@ int cap_file_mmap(struct file *file, unsigned long reqprot, > if (ret == 0) > current->flags |= PF_SUPERPRIV; > } > +#endif > return ret; > } > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index bb230d5..93d61f8 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3046,6 +3046,7 @@ static int selinux_file_mmap(struct file *file, unsigned long reqprot, > unsigned long addr, unsigned long addr_only) > { > int rc = 0; > +#ifdef CONFIG_MMU > u32 sid = current_sid(); > > /* > @@ -3060,6 +3061,7 @@ static int selinux_file_mmap(struct file *file, unsigned long reqprot, > if (rc) > return rc; > } > +#endif > > /* do DAC check on address space usage */ > rc = cap_file_mmap(file, reqprot, prot, flags, addr, addr_only); >
-- -Graff -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |