From: Sumit Garg via OP-TEE <op-tee@lists.trustedfirmware.org>
To: Jens Wiklander <jens.wiklander@linaro.org>,
Masami Ichikawa <masami256@gmail.com>
Cc: op-tee@lists.trustedfirmware.org
Subject: Re: [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages
Date: Fri, 19 Sep 2025 11:35:51 +0530 [thread overview]
Message-ID: <aMzyv0BKFOiE9Jgm@sumit-X1> (raw)
In-Reply-To: <20250918122541.GA2693176@rayden>
On Thu, Sep 18, 2025 at 02:25:41PM +0200, Jens Wiklander wrote:
> Hi Masami,
>
> [+Sumit in CC]
>
> On Wed, Sep 17, 2025 at 10:58:11PM +0900, Masami Ichikawa wrote:
> [snip]
> > I wrote a test program and ran it on both 6.17-rc5 and 6.14. I was
> > able to reproduce the crash on both kernels.
> >
> > I uploaded test code and test results to my gist.
> > https://gist.github.com/masami256/11e21a7503812af7ee1e890080093a2c
> >
> > The test code is crash_test.c. This program takes 2 arguments. First
> > argument is malicious buffer size and second one is actual buffer
> > size.
> > I can reproduce the crash with the following pair.
> >
> > malicious buffer size: 0xffffff
> > actual buffer size: 0xff
>
Thanks Masami for the report and the bug reproducer here.
> Thanks, that easily reproduces the problem. The following diff should fix it:
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -318,7 +318,16 @@ register_shm_helper(struct tee_context *ctx, struct iov_iter *iter, u32 flags,
>
> len = iov_iter_extract_pages(iter, &shm->pages, LONG_MAX, num_pages, 0,
> &off);
> - if (unlikely(len <= 0)) {
> + if (unlikely(len < num_pages * PAGE_SIZE)) {
> + if (len > 0) {
> + /*
> + * If we only got a few pages, update to release
> + * the correct amount below.
> + */
> + shm->num_pages = len / PAGE_SIZE;
> + ret = ERR_PTR(-ENOMEM);
> + goto err_put_shm_pages;
> + }
> ret = len ? ERR_PTR(len) : ERR_PTR(-ENOMEM);
> goto err_free_shm_pages;
> }
Thanks Jens for the fix, it sounds appropriate to me. I think this
commit [1] introduced the bug in the first place as earlier check for
pin_user_pages_fast() would have caught this issue without crashing the
kernel.
Jens, can you please send a proper fix here? I hope we should be able to
get it merged for v6.17 since it sounds critical to me.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7bdee41575919773818e525ea19e54eb817770af
-Sumit
prev parent reply other threads:[~2025-09-19 6:06 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-13 6:06 [BUG] tee_shm: NULL pointer dereference in unpin_user_pages() on invalid shm pages Masami Ichikawa
2025-09-16 7:46 ` Jens Wiklander
2025-09-17 13:58 ` Masami Ichikawa
2025-09-18 12:25 ` Jens Wiklander
2025-09-18 14:31 ` Masami Ichikawa
2025-09-19 6:05 ` Sumit Garg via OP-TEE [this message]
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=aMzyv0BKFOiE9Jgm@sumit-X1 \
--to=op-tee@lists.trustedfirmware.org \
--cc=jens.wiklander@linaro.org \
--cc=masami256@gmail.com \
--cc=sumit.garg@kernel.org \
/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