From: Stefan Berger <stefanb@linux.ibm.com>
To: Roberto Sassu <roberto.sassu@huaweicloud.com>,
zohar@linux.ibm.com, dmitry.kasatkin@gmail.com,
jmorris@namei.org, serge@hallyn.com
Cc: linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, viro@zeniv.linux.org.uk,
Roberto Sassu <roberto.sassu@huawei.com>,
stable@vger.kernel.org
Subject: Re: [PATCH v3 1/2] ima: Align ima_file_mmap() parameters with mmap_file LSM hook
Date: Thu, 26 Jan 2023 14:37:06 -0500 [thread overview]
Message-ID: <295c5915-b680-16be-3d51-b7c1d2ca5e4f@linux.ibm.com> (raw)
In-Reply-To: <20230126163812.1870942-1-roberto.sassu@huaweicloud.com>
On 1/26/23 11:38, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Commit 98de59bfe4b2f ("take calculation of final prot in
> security_mmap_file() into a helper") moved the code to update prot, to be
> the actual protections applied to the kernel, to a new helper called
> mmap_prot().
>
> However, while without the helper ima_file_mmap() was getting the updated
> prot, with the helper ima_file_mmap() gets the original prot, which
> contains the protections requested by the application.
>
> A possible consequence of this change is that, if an application calls
> mmap() with only PROT_READ, and the kernel applies PROT_EXEC in addition,
> that application would have access to executable memory without having this
> event recorded in the IMA measurement list. This situation would occur for
> example if the application, before mmap(), calls the personality() system
> call with READ_IMPLIES_EXEC as the first argument.
>
> Align ima_file_mmap() parameters with those of the mmap_file LSM hook, so
> that IMA can receive both the requested prot and the final prot. Since the
> requested protections are stored in a new variable, and the final
> protections are stored in the existing variable, this effectively restores
> the original behavior of the MMAP_CHECK hook.
>
And flags is being passed in preparation for IMA to meet the interface
requirements of the LSM hooks - I suppose in preparation for IMA to become an LSM.
> Cc: stable@vger.kernel.org
> Fixes: 98de59bfe4b2 ("take calculation of final prot in security_mmap_file() into a helper")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> include/linux/ima.h | 6 ++++--
> security/integrity/ima/ima_main.c | 7 +++++--
> security/security.c | 7 ++++---
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 5a0b2a285a18..d79fee67235e 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -21,7 +21,8 @@ extern int ima_file_check(struct file *file, int mask);
> extern void ima_post_create_tmpfile(struct user_namespace *mnt_userns,
> struct inode *inode);
> extern void ima_file_free(struct file *file);
> -extern int ima_file_mmap(struct file *file, unsigned long prot);
> +extern int ima_file_mmap(struct file *file, unsigned long reqprot,
> + unsigned long prot, unsigned long flags);
> extern int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot);
> extern int ima_load_data(enum kernel_load_data_id id, bool contents);
> extern int ima_post_load_data(char *buf, loff_t size,
> @@ -76,7 +77,8 @@ static inline void ima_file_free(struct file *file)
> return;
> }
>
> -static inline int ima_file_mmap(struct file *file, unsigned long prot)
> +static inline int ima_file_mmap(struct file *file, unsigned long reqprot,
> + unsigned long prot, unsigned long flags)
> {
> return 0;
> }
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 377300973e6c..f48f4e694921 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -397,7 +397,9 @@ static int process_measurement(struct file *file, const struct cred *cred,
> /**
> * ima_file_mmap - based on policy, collect/store measurement.
> * @file: pointer to the file to be measured (May be NULL)
> - * @prot: contains the protection that will be applied by the kernel.
> + * @reqprot: protection requested by the application
> + * @prot: protection that will be applied by the kernel
> + * @flags: operational flags
> *
> * Measure files being mmapped executable based on the ima_must_measure()
> * policy decision.
> @@ -405,7 +407,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> * On success return 0. On integrity appraisal error, assuming the file
> * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
> */
> -int ima_file_mmap(struct file *file, unsigned long prot)
> +int ima_file_mmap(struct file *file, unsigned long reqprot,
> + unsigned long prot, unsigned long flags)
> {
> u32 secid;
>
> diff --git a/security/security.c b/security/security.c
> index d1571900a8c7..174afa4fad81 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1661,12 +1661,13 @@ static inline unsigned long mmap_prot(struct file *file, unsigned long prot)
> int security_mmap_file(struct file *file, unsigned long prot,
> unsigned long flags)
> {
> + unsigned long prot_adj = mmap_prot(file, prot);
> int ret;
> - ret = call_int_hook(mmap_file, 0, file, prot,
> - mmap_prot(file, prot), flags);
> +
> + ret = call_int_hook(mmap_file, 0, file, prot, prot_adj, flags);
> if (ret)
> return ret;
> - return ima_file_mmap(file, prot);
> + return ima_file_mmap(file, prot, prot_adj, flags);
> }
>
> int security_mmap_addr(unsigned long addr)
next prev parent reply other threads:[~2023-01-26 19:37 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-26 16:38 [PATCH v3 1/2] ima: Align ima_file_mmap() parameters with mmap_file LSM hook Roberto Sassu
2023-01-26 19:37 ` Stefan Berger [this message]
2023-01-27 7:55 ` Roberto Sassu
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=295c5915-b680-16be-3d51-b7c1d2ca5e4f@linux.ibm.com \
--to=stefanb@linux.ibm.com \
--cc=dmitry.kasatkin@gmail.com \
--cc=jmorris@namei.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=roberto.sassu@huawei.com \
--cc=roberto.sassu@huaweicloud.com \
--cc=serge@hallyn.com \
--cc=stable@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
--cc=zohar@linux.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).