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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.