On Thu, Mar 10, 2016 at 12:52:35PM +0100, Marc Espie wrote:
> Already shown to a few people, but since pledge(2) aborts on non-dev, let's
> check upfront that we're of the right type.
>
> I don't think this requires a bump. It doesn't really change the interface,
> just makes it stricter.
>
If we are going to fix this in userland, I think espie@'s patch is the
way to go. I like the idea that opendev(3) validates the file descriptor
it returns.
Note that we should support DIOCMAP/DIOCGDINFO in pledge disklabel since
otherwise fsck(8) or mount(8) would need some pledge related fixing
because of their readlabelfs(3) calls.
Given that DIOCMAP is allowed by pledge, pledge disklabel together with
stdio ?path permits all paths through opendev(3), as far as I can see.
Thus, I see no real problem with pledging a program before opendev(3).
dumpfs(8), as it is currently written, really needs opendev(3) to be
allowed by pledge.
Here's another case of a program that would be fixed by espie's patch:
Try
$ fsirand -p /altroot
or, for fun, another variant:
$ touch abcdabcdabcdabcd.a
$ fsirand -p abcdabcdabcdabcd.a
> Index: opendev.3
> ===================================================================
> RCS file: /build/data/openbsd/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 10 Mar 2016 11:51:27 -0000
> @@ -93,7 +93,9 @@ it is modified to point at the fully exp
> The
> .Fn opendev
> return value and errors are the same as the return value and errors of
> -.Xr open 2 .
> +.Xr open 2 ,
> +plus
> +.Er EFTYPE .
> .Sh SEE ALSO
> .Xr open 2 ,
> .Xr getrawpartition 3 ,
> Index: opendev.c
> ===================================================================
> RCS file: /build/data/openbsd/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 8 Mar 2016 22:30:44 -0000
> @@ -38,6 +38,7 @@
> #include <sys/limits.h>
> #include <sys/disk.h>
> #include <sys/dkio.h>
> +#include <sys/stat.h>
>
> #include "util.h"
>
> @@ -50,6 +51,7 @@ opendev(const char *path, int oflags, in
> {
> static char namebuf[PATH_MAX];
> struct dk_diskmap dm;
> + struct stat st;
> char *slash, *prefix;
> int fd;
>
> @@ -106,5 +108,17 @@ opendev(const char *path, int oflags, in
> if (realpath)
> *realpath = namebuf;
>
> + if (fd != -1) {
> + if (fstat(fd, &st) == -1) {
> + close(fd);
> + return (-1);
> + }
> + if ((dflags & OPENDEV_BLCK) ? !S_ISBLK(st.st_mode) :
> + !S_ISCHR(st.st_mode)) {
> + close(fd);
> + errno = EFTYPE;
> + return (-1);
> + }
> + }
> return (fd);
> }
>
