On Thu, Aug 25, 2022 at 05:36:53AM +0000, Klemens Nanni wrote:
> On Wed, Aug 24, 2022 at 08:02:03PM -0600, Todd C. Miller wrote:
> > On Wed, 24 Aug 2022 20:06:00 -0000, Klemens Nanni wrote:
> >
> > > Feedback? Am I missing anything?
> >
> > If fstat(2) fails you should not try to access sb. Perhaps:
> >
> > if (((dflags & OPENDEV_BLCK) && ...
> >
> > should be an "else if (..."
>
> Ah yes, the failure check does not return early but falls through, so
> all further logic needs to check fd and/or errno (like the isduid() case
> already does).
> + } else if (((dflags & OPENDEV_BLCK) &&
> + !S_ISBLK(sb.st_mode)) ||
> + !S_ISCHR(sb.st_mode)) {
And this should use the ternary operator, as this would still succeed
if path is a character device even though OPENDEV_BLCK was passed.
OK?
Index: opendev.3
===================================================================
RCS file: /cvs/src/lib/libutil/opendev.3,v
retrieving revision 1.22
diff -u -p -r1.22 opendev.3
--- opendev.3 15 Jan 2015 19:06:32 -0000 1.22
+++ opendev.3 24 Aug 2022 19:34:20 -0000
@@ -90,10 +90,12 @@ is not
.Dv NULL ,
it is modified to point at the fully expanded device name.
.Sh RETURN VALUES
-The
+If successful,
.Fn opendev
-return value and errors are the same as the return value and errors of
-.Xr open 2 .
+returns a file descriptor.
+Otherwise, a value of -1 is returned and
+.Va errno
+is set to indicate the error.
.Sh SEE ALSO
.Xr open 2 ,
.Xr getrawpartition 3 ,
Index: opendev.c
===================================================================
RCS file: /cvs/src/lib/libutil/opendev.c,v
retrieving revision 1.15
diff -u -p -r1.15 opendev.c
--- opendev.c 30 Jun 2011 15:04:58 -0000 1.15
+++ opendev.c 25 Aug 2022 11:46:26 -0000
@@ -38,6 +38,7 @@
#include <sys/limits.h>
#include <sys/disk.h>
#include <sys/dkio.h>
+#include <sys/stat.h>
#include "util.h"
@@ -63,8 +64,23 @@ opendev(const char *path, int oflags, in
prefix = "r"; /* character device */
if ((slash = strchr(path, '/'))) {
+ struct stat sb;
+
strlcpy(namebuf, path, sizeof(namebuf));
fd = open(namebuf, oflags);
+
+ if (fd != -1) {
+ if (fstat(fd, &sb) == -1) {
+ close(fd);
+ fd = -1;
+ } else if ((dflags & OPENDEV_BLCK) ?
+ !S_ISBLK(sb.st_mode) :
+ !S_ISCHR(sb.st_mode)) {
+ close(fd);
+ fd = -1;
+ errno = ENOTBLK;
+ }
+ }
} else if (isduid(path, dflags)) {
strlcpy(namebuf, path, sizeof(namebuf));
if ((fd = open("/dev/diskmap", oflags)) != -1) {
