| Message ID | 20240618010653.2346969-1-pobrn@protonmail.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2024-06-18 02:06:55) > This commit moves the check that determines whether the > mode argument of `open*()` exists into a separate function. > > With that, the check is fixed because previously it failed to > account for the fact that `O_TMPFILE` is not a power of two. > > Furthermore, add `asserts()` in the fortified variants that > ensure that no mode is required by the specified flags. > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > src/v4l2/v4l2_compat.cpp | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp > index 8a44403e..5c6f925e 100644 > --- a/src/v4l2/v4l2_compat.cpp > +++ b/src/v4l2/v4l2_compat.cpp > @@ -7,6 +7,7 @@ > > #include "v4l2_compat_manager.h" > > +#include <assert.h> > #include <errno.h> > #include <fcntl.h> > #include <stdarg.h> > @@ -28,12 +29,17 @@ using namespace libcamera; > va_end(ap); \ > } > I'd be tempted to add a comment here explaining /what/ this check is for. It wasn't obvious to me straight away. /* * Determine if the flags set will require a further mode flag to be * parsed as an additional argument in the va_args. */ > +static inline bool needs_mode(int flags) > +{ > + return (flags & O_CREAT) || ((flags & O_TMPFILE) == O_TMPFILE); > +} > + > extern "C" { > > LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) > { > mode_t mode = 0; > - if (oflag & O_CREAT || oflag & O_TMPFILE) > + if (needs_mode(oflag)) > extract_va_arg(mode_t, mode, oflag); > > return V4L2CompatManager::instance()->openat(AT_FDCWD, path, > @@ -43,6 +49,7 @@ LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) > /* _FORTIFY_SOURCE redirects open to __open_2 */ > LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) > { > + assert(!needs_mode(oflag)); I was going to ask if we should be returning errors here instead, but now I checked and I see we're in _FORTIFY_SOURCE where assertions are perfectly valid and expected even, so I think this is fine. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > return open(path, oflag); > } > > @@ -50,7 +57,7 @@ LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) > LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) > { > mode_t mode = 0; > - if (oflag & O_CREAT || oflag & O_TMPFILE) > + if (needs_mode(oflag)) > extract_va_arg(mode_t, mode, oflag); > > return V4L2CompatManager::instance()->openat(AT_FDCWD, path, > @@ -59,6 +66,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) > > LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) > { > + assert(!needs_mode(oflag)); > return open64(path, oflag); > } > #endif > @@ -66,7 +74,7 @@ LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) > LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) > { > mode_t mode = 0; > - if (oflag & O_CREAT || oflag & O_TMPFILE) > + if (needs_mode(oflag)) > extract_va_arg(mode_t, mode, oflag); > > return V4L2CompatManager::instance()->openat(dirfd, path, oflag, mode); > @@ -74,6 +82,7 @@ LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) > > LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) > { > + assert(!needs_mode(oflag)); > return openat(dirfd, path, oflag); > } > > @@ -81,7 +90,7 @@ LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) > LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) > { > mode_t mode = 0; > - if (oflag & O_CREAT || oflag & O_TMPFILE) > + if (needs_mode(oflag)) > extract_va_arg(mode_t, mode, oflag); > > return V4L2CompatManager::instance()->openat(dirfd, path, > @@ -90,6 +99,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) > > LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag) > { > + assert(!needs_mode(oflag)); > return openat64(dirfd, path, oflag); > } > #endif > -- > 2.45.2 > >
On Fri, Jun 21, 2024 at 12:02:49PM +0100, Kieran Bingham wrote: > Quoting Barnabás Pőcze (2024-06-18 02:06:55) > > This commit moves the check that determines whether the > > mode argument of `open*()` exists into a separate function. > > > > With that, the check is fixed because previously it failed to > > account for the fact that `O_TMPFILE` is not a power of two. > > > > Furthermore, add `asserts()` in the fortified variants that > > ensure that no mode is required by the specified flags. > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > --- > > src/v4l2/v4l2_compat.cpp | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp > > index 8a44403e..5c6f925e 100644 > > --- a/src/v4l2/v4l2_compat.cpp > > +++ b/src/v4l2/v4l2_compat.cpp > > @@ -7,6 +7,7 @@ > > > > #include "v4l2_compat_manager.h" > > > > +#include <assert.h> > > #include <errno.h> > > #include <fcntl.h> > > #include <stdarg.h> > > @@ -28,12 +29,17 @@ using namespace libcamera; > > va_end(ap); \ > > } > > > > I'd be tempted to add a comment here explaining /what/ this check is > for. It wasn't obvious to me straight away. > > > /* > * Determine if the flags set will require a further mode flag to be > * parsed as an additional argument in the va_args. I had a bit of trouble parsing this. If you think the following is better you can use it. * Determine if the flags require a further mode arguments that needs to be * parsed from va_args. > */ > > > +static inline bool needs_mode(int flags) Maybe open_needs_mode() to match the name of the macro in fcntl.h ? Up to you. > > +{ > > + return (flags & O_CREAT) || ((flags & O_TMPFILE) == O_TMPFILE); > > +} > > + > > extern "C" { > > > > LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) > > { > > mode_t mode = 0; > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > + if (needs_mode(oflag)) > > extract_va_arg(mode_t, mode, oflag); > > > > return V4L2CompatManager::instance()->openat(AT_FDCWD, path, > > @@ -43,6 +49,7 @@ LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) > > /* _FORTIFY_SOURCE redirects open to __open_2 */ > > LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) > > { > > + assert(!needs_mode(oflag)); > > I was going to ask if we should be returning errors here instead, but > now I checked and I see we're in _FORTIFY_SOURCE where assertions are > perfectly valid and expected even, so I think this is fine. Is this check needed though ? FORTIFY_SOURCE already catches this issue, so I don't think we can be called with O_CREAT or O_TMPFILE here, the code wouldn't have compiled in the first place. Of course someone could call __open_2() manually without going through the FORTIFY_SOURCE open() wrapper, but in that case I think they don't deserve us caring :-) > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > return open(path, oflag); > > } > > > > @@ -50,7 +57,7 @@ LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) > > LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) > > { > > mode_t mode = 0; > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > + if (needs_mode(oflag)) > > extract_va_arg(mode_t, mode, oflag); > > > > return V4L2CompatManager::instance()->openat(AT_FDCWD, path, > > @@ -59,6 +66,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) > > > > LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) > > { > > + assert(!needs_mode(oflag)); > > return open64(path, oflag); > > } > > #endif > > @@ -66,7 +74,7 @@ LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) > > LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) > > { > > mode_t mode = 0; > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > + if (needs_mode(oflag)) > > extract_va_arg(mode_t, mode, oflag); > > > > return V4L2CompatManager::instance()->openat(dirfd, path, oflag, mode); > > @@ -74,6 +82,7 @@ LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) > > > > LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) > > { > > + assert(!needs_mode(oflag)); > > return openat(dirfd, path, oflag); > > } > > > > @@ -81,7 +90,7 @@ LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) > > LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) > > { > > mode_t mode = 0; > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > + if (needs_mode(oflag)) > > extract_va_arg(mode_t, mode, oflag); > > > > return V4L2CompatManager::instance()->openat(dirfd, path, > > @@ -90,6 +99,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) > > > > LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag) > > { > > + assert(!needs_mode(oflag)); > > return openat64(dirfd, path, oflag); > > } > > #endif > >
> On Fri, Jun 21, 2024 at 12:02:49PM +0100, Kieran Bingham wrote: > > Quoting Barnabás Pőcze (2024-06-18 02:06:55) > > > This commit moves the check that determines whether the > > > mode argument of `open*()` exists into a separate function. > > > > > > With that, the check is fixed because previously it failed to > > > account for the fact that `O_TMPFILE` is not a power of two. > > > > > > Furthermore, add `asserts()` in the fortified variants that > > > ensure that no mode is required by the specified flags. > > > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > > --- > > > src/v4l2/v4l2_compat.cpp | 18 ++++++++++++++---- > > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp > > > index 8a44403e..5c6f925e 100644 > > > --- a/src/v4l2/v4l2_compat.cpp > > > +++ b/src/v4l2/v4l2_compat.cpp > > > @@ -7,6 +7,7 @@ > > > > > > #include "v4l2_compat_manager.h" > > > > > > +#include <assert.h> > > > #include <errno.h> > > > #include <fcntl.h> > > > #include <stdarg.h> > > > @@ -28,12 +29,17 @@ using namespace libcamera; > > > va_end(ap); \ > > > } > > > > > > > I'd be tempted to add a comment here explaining /what/ this check is > > for. It wasn't obvious to me straight away. > > > > > > /* > > * Determine if the flags set will require a further mode flag to be > > * parsed as an additional argument in the va_args. > > I had a bit of trouble parsing this. If you think the following is > better you can use it. > > * Determine if the flags require a further mode arguments that needs to be > * parsed from va_args. > > > */ > > > > > +static inline bool needs_mode(int flags) > > Maybe open_needs_mode() to match the name of the macro in fcntl.h ? Up > to you. If someone wants to rename the function or add a comment while committing, I am fine with that. Or should I send a new version? > > > > +{ > > > + return (flags & O_CREAT) || ((flags & O_TMPFILE) == O_TMPFILE); > > > +} > > > + > > > extern "C" { > > > > > > LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) > > > { > > > mode_t mode = 0; > > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > > + if (needs_mode(oflag)) > > > extract_va_arg(mode_t, mode, oflag); > > > > > > return V4L2CompatManager::instance()->openat(AT_FDCWD, path, > > > @@ -43,6 +49,7 @@ LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) > > > /* _FORTIFY_SOURCE redirects open to __open_2 */ > > > LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) > > > { > > > + assert(!needs_mode(oflag)); > > > > I was going to ask if we should be returning errors here instead, but > > now I checked and I see we're in _FORTIFY_SOURCE where assertions are > > perfectly valid and expected even, so I think this is fine. > > Is this check needed though ? FORTIFY_SOURCE already catches this issue, > so I don't think we can be called with O_CREAT or O_TMPFILE here, the > code wouldn't have compiled in the first place. Of course someone could > call __open_2() manually without going through the FORTIFY_SOURCE open() > wrapper, but in that case I think they don't deserve us caring :-) Well, glibc has a check as well: https://sourceware.org/git/?p=glibc.git;a=blob;f=io/open_2.c;h=6466c36f5adeef9c59b494bd4de2fe30c740a4b4;hb=HEAD#l26 So I don't think it hurts. Regards, Barnabás Pőcze > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > return open(path, oflag); > > > } > > > > > > @@ -50,7 +57,7 @@ LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) > > > LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) > > > { > > > mode_t mode = 0; > > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > > + if (needs_mode(oflag)) > > > extract_va_arg(mode_t, mode, oflag); > > > > > > return V4L2CompatManager::instance()->openat(AT_FDCWD, path, > > > @@ -59,6 +66,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) > > > > > > LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) > > > { > > > + assert(!needs_mode(oflag)); > > > return open64(path, oflag); > > > } > > > #endif > > > @@ -66,7 +74,7 @@ LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) > > > LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) > > > { > > > mode_t mode = 0; > > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > > + if (needs_mode(oflag)) > > > extract_va_arg(mode_t, mode, oflag); > > > > > > return V4L2CompatManager::instance()->openat(dirfd, path, oflag, mode); > > > @@ -74,6 +82,7 @@ LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) > > > > > > LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) > > > { > > > + assert(!needs_mode(oflag)); > > > return openat(dirfd, path, oflag); > > > } > > > > > > @@ -81,7 +90,7 @@ LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) > > > LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) > > > { > > > mode_t mode = 0; > > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > > + if (needs_mode(oflag)) > > > extract_va_arg(mode_t, mode, oflag); > > > > > > return V4L2CompatManager::instance()->openat(dirfd, path, > > > @@ -90,6 +99,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) > > > > > > LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag) > > > { > > > + assert(!needs_mode(oflag)); > > > return openat64(dirfd, path, oflag); > > > } > > > #endif > > > > > -- > Regards, > > Laurent Pinchart >
Hi Barnabás, On Fri, Jun 21, 2024 at 01:55:43PM +0000, Barnabás Pőcze wrote: > > On Fri, Jun 21, 2024 at 12:02:49PM +0100, Kieran Bingham wrote: > > > Quoting Barnabás Pőcze (2024-06-18 02:06:55) > > > > This commit moves the check that determines whether the > > > > mode argument of `open*()` exists into a separate function. > > > > > > > > With that, the check is fixed because previously it failed to > > > > account for the fact that `O_TMPFILE` is not a power of two. > > > > > > > > Furthermore, add `asserts()` in the fortified variants that > > > > ensure that no mode is required by the specified flags. > > > > > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > > > --- > > > > src/v4l2/v4l2_compat.cpp | 18 ++++++++++++++---- > > > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp > > > > index 8a44403e..5c6f925e 100644 > > > > --- a/src/v4l2/v4l2_compat.cpp > > > > +++ b/src/v4l2/v4l2_compat.cpp > > > > @@ -7,6 +7,7 @@ > > > > > > > > #include "v4l2_compat_manager.h" > > > > > > > > +#include <assert.h> > > > > #include <errno.h> > > > > #include <fcntl.h> > > > > #include <stdarg.h> > > > > @@ -28,12 +29,17 @@ using namespace libcamera; > > > > va_end(ap); \ > > > > } > > > > > > > > > > I'd be tempted to add a comment here explaining /what/ this check is > > > for. It wasn't obvious to me straight away. > > > > > > /* > > > * Determine if the flags set will require a further mode flag to be > > > * parsed as an additional argument in the va_args. > > > > I had a bit of trouble parsing this. If you think the following is > > better you can use it. > > > > * Determine if the flags require a further mode arguments that needs to be > > * parsed from va_args. > > > > > */ > > > > > > > +static inline bool needs_mode(int flags) > > > > Maybe open_needs_mode() to match the name of the macro in fcntl.h ? Up > > to you. > > If someone wants to rename the function or add a comment while committing, I am fine with that. > Or should I send a new version? I'll add the comment, no need to resend. > > > > +{ > > > > + return (flags & O_CREAT) || ((flags & O_TMPFILE) == O_TMPFILE); > > > > +} > > > > + > > > > extern "C" { > > > > > > > > LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) > > > > { > > > > mode_t mode = 0; > > > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > > > + if (needs_mode(oflag)) > > > > extract_va_arg(mode_t, mode, oflag); > > > > > > > > return V4L2CompatManager::instance()->openat(AT_FDCWD, path, > > > > @@ -43,6 +49,7 @@ LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) > > > > /* _FORTIFY_SOURCE redirects open to __open_2 */ > > > > LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) > > > > { > > > > + assert(!needs_mode(oflag)); > > > > > > I was going to ask if we should be returning errors here instead, but > > > now I checked and I see we're in _FORTIFY_SOURCE where assertions are > > > perfectly valid and expected even, so I think this is fine. > > > > Is this check needed though ? FORTIFY_SOURCE already catches this issue, > > so I don't think we can be called with O_CREAT or O_TMPFILE here, the > > code wouldn't have compiled in the first place. Of course someone could > > call __open_2() manually without going through the FORTIFY_SOURCE open() > > wrapper, but in that case I think they don't deserve us caring :-) > > Well, glibc has a check as well: https://sourceware.org/git/?p=glibc.git;a=blob;f=io/open_2.c;h=6466c36f5adeef9c59b494bd4de2fe30c740a4b4;hb=HEAD#l26 > So I don't think it hurts. If glibc has those checks we may not need to duplicate them :-) But I'm OK with the asserts. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > return open(path, oflag); > > > > } > > > > > > > > @@ -50,7 +57,7 @@ LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) > > > > LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) > > > > { > > > > mode_t mode = 0; > > > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > > > + if (needs_mode(oflag)) > > > > extract_va_arg(mode_t, mode, oflag); > > > > > > > > return V4L2CompatManager::instance()->openat(AT_FDCWD, path, > > > > @@ -59,6 +66,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) > > > > > > > > LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) > > > > { > > > > + assert(!needs_mode(oflag)); > > > > return open64(path, oflag); > > > > } > > > > #endif > > > > @@ -66,7 +74,7 @@ LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) > > > > LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) > > > > { > > > > mode_t mode = 0; > > > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > > > + if (needs_mode(oflag)) > > > > extract_va_arg(mode_t, mode, oflag); > > > > > > > > return V4L2CompatManager::instance()->openat(dirfd, path, oflag, mode); > > > > @@ -74,6 +82,7 @@ LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) > > > > > > > > LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) > > > > { > > > > + assert(!needs_mode(oflag)); > > > > return openat(dirfd, path, oflag); > > > > } > > > > > > > > @@ -81,7 +90,7 @@ LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) > > > > LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) > > > > { > > > > mode_t mode = 0; > > > > - if (oflag & O_CREAT || oflag & O_TMPFILE) > > > > + if (needs_mode(oflag)) > > > > extract_va_arg(mode_t, mode, oflag); > > > > > > > > return V4L2CompatManager::instance()->openat(dirfd, path, > > > > @@ -90,6 +99,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) > > > > > > > > LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag) > > > > { > > > > + assert(!needs_mode(oflag)); > > > > return openat64(dirfd, path, oflag); > > > > } > > > > #endif
diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp index 8a44403e..5c6f925e 100644 --- a/src/v4l2/v4l2_compat.cpp +++ b/src/v4l2/v4l2_compat.cpp @@ -7,6 +7,7 @@ #include "v4l2_compat_manager.h" +#include <assert.h> #include <errno.h> #include <fcntl.h> #include <stdarg.h> @@ -28,12 +29,17 @@ using namespace libcamera; va_end(ap); \ } +static inline bool needs_mode(int flags) +{ + return (flags & O_CREAT) || ((flags & O_TMPFILE) == O_TMPFILE); +} + extern "C" { LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) { mode_t mode = 0; - if (oflag & O_CREAT || oflag & O_TMPFILE) + if (needs_mode(oflag)) extract_va_arg(mode_t, mode, oflag); return V4L2CompatManager::instance()->openat(AT_FDCWD, path, @@ -43,6 +49,7 @@ LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...) /* _FORTIFY_SOURCE redirects open to __open_2 */ LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) { + assert(!needs_mode(oflag)); return open(path, oflag); } @@ -50,7 +57,7 @@ LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag) LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) { mode_t mode = 0; - if (oflag & O_CREAT || oflag & O_TMPFILE) + if (needs_mode(oflag)) extract_va_arg(mode_t, mode, oflag); return V4L2CompatManager::instance()->openat(AT_FDCWD, path, @@ -59,6 +66,7 @@ LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...) LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) { + assert(!needs_mode(oflag)); return open64(path, oflag); } #endif @@ -66,7 +74,7 @@ LIBCAMERA_PUBLIC int __open64_2(const char *path, int oflag) LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) { mode_t mode = 0; - if (oflag & O_CREAT || oflag & O_TMPFILE) + if (needs_mode(oflag)) extract_va_arg(mode_t, mode, oflag); return V4L2CompatManager::instance()->openat(dirfd, path, oflag, mode); @@ -74,6 +82,7 @@ LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...) LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) { + assert(!needs_mode(oflag)); return openat(dirfd, path, oflag); } @@ -81,7 +90,7 @@ LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag) LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) { mode_t mode = 0; - if (oflag & O_CREAT || oflag & O_TMPFILE) + if (needs_mode(oflag)) extract_va_arg(mode_t, mode, oflag); return V4L2CompatManager::instance()->openat(dirfd, path, @@ -90,6 +99,7 @@ LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...) LIBCAMERA_PUBLIC int __openat64_2(int dirfd, const char *path, int oflag) { + assert(!needs_mode(oflag)); return openat64(dirfd, path, oflag); } #endif
This commit moves the check that determines whether the mode argument of `open*()` exists into a separate function. With that, the check is fixed because previously it failed to account for the fact that `O_TMPFILE` is not a power of two. Furthermore, add `asserts()` in the fortified variants that ensure that no mode is required by the specified flags. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- src/v4l2/v4l2_compat.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)