From: Dylan Jhong <dylan@andestech.com>
To: "rpalethorpe@suse.de" <rpalethorpe@suse.de>
Cc: "Randolph Sheng-Kai Lin\(\(\(\(\(\(\(\(\(\(\)"
<randolph@andestech.com>,
"ltp@lists.linux.it" <ltp@lists.linux.it>,
"x5710999x@gmail.com" <x5710999x@gmail.com>,
"Alan Quey-Liang Kao\(\(\(\(\(\(\(\(\(\(\)"
<alankao@andestech.com>
Subject: Re: [LTP] [PATCH] syscalls/semctl03: Solve kernel panic in semctl03
Date: Fri, 26 Aug 2022 15:41:15 +0800 [thread overview]
Message-ID: <Ywh5G6RQi+zitagg@atcsi01> (raw)
In-Reply-To: <87k06v1pwp.fsf@suse.de>
Hi Richard,
Thanks for your reply.
My opinion is the same as yours, libc should do more checking and protection for incoming parameters
In semctl03.c, the two tv->semctl() implementation functions, which are libc_semctl() and sys_semctl(),
do not pass the 4th argument ".buf" to the next level system call.
At present, the 4th argument of semctl() implemented in semctl03.c is hard-coded,
I think passing parameters instead of hardcoding should be more better for this testcase.
Should we pass all parameters to the next level semctl() system call?
Partial code of semctl03.c:
--------------------------------------------------------
TST_EXP_FAIL(tv->semctl(*(tc->sem_id), 0, tc->ipc_cmd, *(tc->buf)), <--- Pass *(tc->buf) to tv->semctl()
tc->error, "semctl() with %s", tc->message);
static union semun arg = {0};
static int libc_semctl(int semid, int semnum, int cmd, ...)
{
return semctl(semid, semnum, cmd, arg); <----- Ignore the 4th parameter and use the hard-coded "arg" directly
}
--------------------------------------------------------
ref:
https://lists.linux.it/pipermail/ltp/2021-June/023116.html
Best,
Dylan
On Fri, Aug 26, 2022 at 02:12:19PM +0800, Richard Palethorpe wrote:
> Hello,
>
> Dylan Jhong <dylan@andestech.com> writes:
>
> > When using semctl() through glibc and __IPC_TIME64 is defined, glibc will
> > call a converted semun64_to_ksemun64() function[*1]. If the parameter of
> > this function is NULL, it will cause a NULL pointer dereference kernel
> > panic.
>
> This is a kernel bug. Generally speaking, we shouldn't be able to create
> kernel panics from user land. The kernel should return EFAULT if we pass
> an invalid pointer.
>
> If this test causes a kernel panic then it should be kept as-is. If it
> is not testing what it was originally intended to, then another test can
> be created to do that.
>
> >
> > In semctl03.c, we need to ensure the element "struct semid_ds *buf" in 4th
> > parameter "union semun" in semctl() is not NULL. But the 4th parameters of
> > libc_semctl() and sys_semctl() are hard-coded[*2] and the element
> > "struct semid_ds *buf" is not given an initial value. Using va_list to pass
> > the correct parameters can solve the problem.
> >
> > ref:
> > [*1]: https://github.com/bminor/glibc/blob/f94f6d8a3572840d3ba42ab9ace3ea522c99c0c2/sysdeps/unix/sysv/linux/semctl.c#L172
> > [*2]: https://github.com/linux-test-project/ltp/blob/58caa8cca507133ea92bd0ea277b91add96e72af/testcases/kernel/syscalls/ipc/semctl/semctl03.c#L45
> >
> > Co-developed-by: Randolph <randolph@andestech.com>
> > Signed-off-by: Dylan Jhong <dylan@andestech.com>
> > ---
> > testcases/kernel/syscalls/ipc/semctl/semctl03.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/testcases/kernel/syscalls/ipc/semctl/semctl03.c b/testcases/kernel/syscalls/ipc/semctl/semctl03.c
> > index a1a4c81ce..bb25053e2 100644
> > --- a/testcases/kernel/syscalls/ipc/semctl/semctl03.c
> > +++ b/testcases/kernel/syscalls/ipc/semctl/semctl03.c
> > @@ -28,11 +28,21 @@ static union semun arg = {0};
> >
> > static int libc_semctl(int semid, int semnum, int cmd, ...)
> > {
> > + va_list ap;
> > +
> > + va_start(ap, cmd);
> > + arg = va_arg(ap, union semun);
> > + va_end(ap);
> > return semctl(semid, semnum, cmd, arg);
> > }
> >
> > static int sys_semctl(int semid, int semnum, int cmd, ...)
> > {
> > + va_list ap;
> > +
> > + va_start(ap, cmd);
> > + arg = va_arg(ap, union semun);
> > + va_end(ap);
> > return tst_syscall(__NR_semctl, semid, semnum, cmd, arg);
> > }
> >
> > --
> > 2.34.1
>
>
> --
> Thank you,
> Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
next prev parent reply other threads:[~2022-08-26 7:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-25 10:52 [LTP] [PATCH] syscalls/semctl03: Solve kernel panic in semctl03 Dylan Jhong
2022-08-26 6:12 ` Richard Palethorpe
2022-08-26 7:41 ` Dylan Jhong [this message]
2022-08-26 7:53 ` Richard Palethorpe
2022-08-29 11:22 ` Dylan Jhong
2022-08-29 14:22 ` Richard Palethorpe
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=Ywh5G6RQi+zitagg@atcsi01 \
--to=dylan@andestech.com \
--cc=alankao@andestech.com \
--cc=ltp@lists.linux.it \
--cc=randolph@andestech.com \
--cc=rpalethorpe@suse.de \
--cc=x5710999x@gmail.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).