| Message ID | 6f9470e3-389d-4cc8-f98f-de9f78ea6e6e@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Avoid uninitialized result in sem_open when file does not exist | expand |
On Fri, Nov 8, 2024 at 8:30 AM Joseph Myers <josmyers@redhat.com> wrote: > > A static analyzer apparently reported an uninitialized use of the > variable result in sem_open in the case where the file is required to > exist but does not exist. > > The report appears to be correct; set result to SEM_FAILED in that > case, and add a test for it. > > Note: the test passes for me even without the sem_open fix, I guess > because result happens to get value SEM_FAILED (i.e. 0) when > uninitialized. > > Tested for x86_64. > > diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile > index 4c1dc04b20..7f52e7d511 100644 > --- a/sysdeps/pthread/Makefile > +++ b/sysdeps/pthread/Makefile > @@ -256,6 +256,7 @@ tests += \ > tst-sem14 \ > tst-sem15 \ > tst-sem16 \ > + tst-sem17 \ > tst-setuid3 \ > tst-signal1 \ > tst-signal2 \ > diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c > index e41236157a..dab734191a 100644 > --- a/sysdeps/pthread/sem_open.c > +++ b/sysdeps/pthread/sem_open.c > @@ -76,6 +76,7 @@ __sem_open (const char *name, int oflag, ...) > goto try_create; > > /* Return. errno is already set. */ > + result = SEM_FAILED; > } > else > /* Check whether we already have this semaphore mapped and > diff --git a/sysdeps/pthread/tst-sem17.c b/sysdeps/pthread/tst-sem17.c > new file mode 100644 > index 0000000000..c3f05d196f > --- /dev/null > +++ b/sysdeps/pthread/tst-sem17.c > @@ -0,0 +1,35 @@ > +/* Test sem_open with missing file. > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <semaphore.h> > + > +#include <support/check.h> > + > +int > +do_test (void) > +{ > + sem_unlink ("/glibc-tst-sem17"); > + errno = 0; > + sem_t *s = sem_open ("/glibc-tst-sem17", 0); > + TEST_VERIFY (s == SEM_FAILED); > + TEST_COMPARE (errno, ENOENT); > + return 0; > +} > + > +#include <support/test-driver.c> > > -- > Joseph S. Myers > josmyers@redhat.com > LGTM. Reviewed-by: H.J. Lu <hjl.tools@gmail.com> Thanks.
On 07/11/24 21:29, Joseph Myers wrote: > A static analyzer apparently reported an uninitialized use of the > variable result in sem_open in the case where the file is required to > exist but does not exist. > > The report appears to be correct; set result to SEM_FAILED in that > case, and add a test for it. > > Note: the test passes for me even without the sem_open fix, I guess > because result happens to get value SEM_FAILED (i.e. 0) when > uninitialized. > > Tested for x86_64. > > diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile > index 4c1dc04b20..7f52e7d511 100644 > --- a/sysdeps/pthread/Makefile > +++ b/sysdeps/pthread/Makefile > @@ -256,6 +256,7 @@ tests += \ > tst-sem14 \ > tst-sem15 \ > tst-sem16 \ > + tst-sem17 \ > tst-setuid3 \ > tst-signal1 \ > tst-signal2 \ We already have a tst-sem17 on nptl, maybe rename it to tst-sem18. > diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c > index e41236157a..dab734191a 100644 > --- a/sysdeps/pthread/sem_open.c > +++ b/sysdeps/pthread/sem_open.c > @@ -76,6 +76,7 @@ __sem_open (const char *name, int oflag, ...) > goto try_create; > > /* Return. errno is already set. */ > + result = SEM_FAILED; > } > else > /* Check whether we already have this semaphore mapped and > diff --git a/sysdeps/pthread/tst-sem17.c b/sysdeps/pthread/tst-sem17.c > new file mode 100644 > index 0000000000..c3f05d196f > --- /dev/null > +++ b/sysdeps/pthread/tst-sem17.c > @@ -0,0 +1,35 @@ > +/* Test sem_open with missing file. > + Copyright (C) 2024 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <semaphore.h> > + > +#include <support/check.h> > + > +int > +do_test (void) > +{ > + sem_unlink ("/glibc-tst-sem17"); > + errno = 0; > + sem_t *s = sem_open ("/glibc-tst-sem17", 0); > + TEST_VERIFY (s == SEM_FAILED); > + TEST_COMPARE (errno, ENOENT); > + return 0; > +} > + > +#include <support/test-driver.c> >
diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index 4c1dc04b20..7f52e7d511 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -256,6 +256,7 @@ tests += \ tst-sem14 \ tst-sem15 \ tst-sem16 \ + tst-sem17 \ tst-setuid3 \ tst-signal1 \ tst-signal2 \ diff --git a/sysdeps/pthread/sem_open.c b/sysdeps/pthread/sem_open.c index e41236157a..dab734191a 100644 --- a/sysdeps/pthread/sem_open.c +++ b/sysdeps/pthread/sem_open.c @@ -76,6 +76,7 @@ __sem_open (const char *name, int oflag, ...) goto try_create; /* Return. errno is already set. */ + result = SEM_FAILED; } else /* Check whether we already have this semaphore mapped and diff --git a/sysdeps/pthread/tst-sem17.c b/sysdeps/pthread/tst-sem17.c new file mode 100644 index 0000000000..c3f05d196f --- /dev/null +++ b/sysdeps/pthread/tst-sem17.c @@ -0,0 +1,35 @@ +/* Test sem_open with missing file. + Copyright (C) 2024 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <semaphore.h> + +#include <support/check.h> + +int +do_test (void) +{ + sem_unlink ("/glibc-tst-sem17"); + errno = 0; + sem_t *s = sem_open ("/glibc-tst-sem17", 0); + TEST_VERIFY (s == SEM_FAILED); + TEST_COMPARE (errno, ENOENT); + return 0; +} + +#include <support/test-driver.c>