Re: video(1) pledge
On Mon, Jan 21, 2019 at 09:44:59PM +0100, Matthieu Herrb wrote:
> On Mon, Jan 21, 2019 at 09:27:57PM +0100, Landry Breuil wrote:
> > Hi,
> >
>
> Hi,
>
> > now that the 'video' promise is in, looking for okays to pledge
> > video(1).
> >
> > with help & hints from semarie@.
>
> One comment in-line.
> >
> > Index: video.c
> > ===
> > RCS file: /cvs/xenocara/app/video/video.c,v
> > retrieving revision 1.25
> > diff -u -r1.25 video.c
> > --- video.c 9 Apr 2018 18:16:44 - 1.25
> > +++ video.c 30 Dec 2018 09:39:27 -
> > @@ -1961,6 +1961,8 @@
> > argv += optind;
> >
> > if (vid.mode & M_QUERY) {
> > + if (pledge("stdio rpath wpath video", NULL) == -1)
> > + err(1, "pledge");
> > dev_dump_query(&vid);
> > cleanup(&vid, 0);
> > }
> > @@ -1970,6 +1972,14 @@
> >
> > if (!setup(&vid))
> > cleanup(&vid, 1);
> > +
> > + if (vid.mode & M_IN_FILE) {
> > + if (pledge("stdio", NULL) == -1)
>
> Like people have found out the hard way recently, X libs need "rpath"
> in case the X error handler needs to be called.
Right, forgot about that issue. new diff :)
Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.26
diff -u -r1.26 video.c
--- video.c 4 Jan 2019 17:45:00 - 1.26
+++ video.c 21 Jan 2019 20:50:06 -
@@ -1961,6 +1961,8 @@
argv += optind;
if (vid.mode & M_QUERY) {
+ if (pledge("stdio rpath wpath video", NULL) == -1)
+ err(1, "pledge");
dev_dump_query(&vid);
cleanup(&vid, 0);
}
@@ -1970,6 +1972,14 @@
if (!setup(&vid))
cleanup(&vid, 1);
+
+ if (vid.mode & M_IN_FILE) {
+ if (pledge("stdio rpath", NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio rpath video", NULL) == -1)
+ err(1, "pledge");
+ }
if (!stream(&vid))
cleanup(&vid, 1);
Re: video(1) pledge
On Mon, Jan 21, 2019 at 09:27:57PM +0100, Landry Breuil wrote:
> Hi,
>
Hi,
> now that the 'video' promise is in, looking for okays to pledge
> video(1).
>
> with help & hints from semarie@.
One comment in-line.
>
> Index: video.c
> ===
> RCS file: /cvs/xenocara/app/video/video.c,v
> retrieving revision 1.25
> diff -u -r1.25 video.c
> --- video.c 9 Apr 2018 18:16:44 - 1.25
> +++ video.c 30 Dec 2018 09:39:27 -
> @@ -1961,6 +1961,8 @@
> argv += optind;
>
> if (vid.mode & M_QUERY) {
> + if (pledge("stdio rpath wpath video", NULL) == -1)
> + err(1, "pledge");
> dev_dump_query(&vid);
> cleanup(&vid, 0);
> }
> @@ -1970,6 +1972,14 @@
>
> if (!setup(&vid))
> cleanup(&vid, 1);
> +
> + if (vid.mode & M_IN_FILE) {
> + if (pledge("stdio", NULL) == -1)
Like people have found out the hard way recently, X libs need "rpath"
in case the X error handler needs to be called.
> + err(1, "pledge");
> + } else {
> + if (pledge("stdio rpath video", NULL) == -1)
> + err(1, "pledge");
> + }
>
> if (!stream(&vid))
> cleanup(&vid, 1);
--
Matthieu Herrb
video(1) pledge
Hi,
now that the 'video' promise is in, looking for okays to pledge
video(1).
with help & hints from semarie@.
Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.25
diff -u -r1.25 video.c
--- video.c 9 Apr 2018 18:16:44 - 1.25
+++ video.c 30 Dec 2018 09:39:27 -
@@ -1961,6 +1961,8 @@
argv += optind;
if (vid.mode & M_QUERY) {
+ if (pledge("stdio rpath wpath video", NULL) == -1)
+ err(1, "pledge");
dev_dump_query(&vid);
cleanup(&vid, 0);
}
@@ -1970,6 +1972,14 @@
if (!setup(&vid))
cleanup(&vid, 1);
+
+ if (vid.mode & M_IN_FILE) {
+ if (pledge("stdio", NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio rpath video", NULL) == -1)
+ err(1, "pledge");
+ }
if (!stream(&vid))
cleanup(&vid, 1);
Re: video(1) pledge (& updated kernel diff)
On Sun, Dec 30, 2018 at 12:14:58PM +0100, Sebastien Marie wrote:
> On Sun, Dec 30, 2018 at 10:58:58AM +0100, Landry Breuil wrote:
> > On Sat, Dec 29, 2018 at 09:30:22AM +0100, Sebastien Marie wrote:
> > > On Fri, Dec 28, 2018 at 09:41:06PM +0100, Landry Breuil wrote:
> > >
> > > I would separate the addition of pledge(2) and unrelated fixes.
> >
> > Right, two separated diffs attached for this.
> >
>
> the err->errs diff to avoid shadowing is ok semarie@
>
> mherrb@, any objections to it ?
Hi,
sorry for not noticing that I was summoned...
No objection.
>
> Thanks
> --
> Sebastien Marie
>
> > Index: video.c
> > ===
> > RCS file: /cvs/xenocara/app/video/video.c,v
> > retrieving revision 1.25
> > diff -u -r1.25 video.c
> > --- video.c 9 Apr 2018 18:16:44 - 1.25
> > +++ video.c 30 Dec 2018 09:39:21 -
> > @@ -1854,7 +1854,7 @@
> > struct xdsp *x = &vid.xdsp;
> > const char *errstr;
> > size_t len;
> > - int ch, err = 0;
> > + int ch, errs = 0;
> >
> > bzero(&vid, sizeof(struct video));
> >
> > @@ -1872,21 +1872,21 @@
> > x->cur_adap = strtonum(optarg, 0, 4, &errstr);
> > if (errstr != NULL) {
> > warnx("Xv adaptor '%s' is %s", optarg, errstr);
> > - err++;
> > + errs++;
> > }
> > break;
> > case 'e':
> > vid.enc = find_enc(optarg);
> > if (vid.enc >= ENC_LAST) {
> > warnx("encoding '%s' is invalid", optarg);
> > - err++;
> > + errs++;
> > }
> > break;
> > case 'f':
> > len = strlcpy(d->path, optarg, sizeof(d->path));
> > if (len >= sizeof(d->path)) {
> > warnx("file path is too long: %s", optarg);
> > - err++;
> > + errs++;
> > }
> > break;
> > case 'g':
> > @@ -1894,8 +1894,8 @@
> > break;
> > case 'i':
> > if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
> > - warnx("only one input or ouput file allowed");
> > - err++;
> > + warnx("only one input or output file allowed");
> > + errs++;
> > } else {
> > vid.mode = (vid.mode & ~M_IN_DEV) | M_IN_FILE;
> > vid.mmap_on = 0; /* mmap mode does not work for
> > files */
> > @@ -1904,15 +1904,15 @@
> > if (len >= sizeof(vid.iofile)) {
> > warnx("input path is too long: %s",
> > optarg);
> > - err++;
> > + errs++;
> > }
> > }
> > break;
> > case 'o':
> > case 'O':
> > if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
> > - warnx("only one input or ouput file allowed");
> > - err++;
> > + warnx("only one input or output file allowed");
> > + errs++;
> > } else {
> > vid.mode |= M_OUT_FILE;
> > if (ch != 'O')
> > @@ -1922,7 +1922,7 @@
> > if (len >= sizeof(vid.iofile)) {
> > warnx("output path is too long: %s",
> > optarg);
> > - err++;
> > + errs++;
> > }
> > }
> > break;
> > @@ -1937,7 +1937,7 @@
> > vid.fps = strtonum(optarg, 1, 100, &errstr);
> > if (errstr != NULL) {
> > warnx("frame rate '%s' is %s", optarg, errstr);
> > - err++;
> > + errs++;
> > }
> > break;
> > case 's':
> > @@ -1947,13 +1947,13 @@
> > vid.verbose++;
> > break;
> > default:
> > - err++;
> > + errs++;
> > break;
> > }
> > - if (err > 0)
> > + if (errs > 0)
> > break;
> > }
> > - if (err > 0) {
> > + if (errs > 0) {
> > usage();
> > cleanup(&vid, 1);
> > }
--
Matthieu Herrb
Re: video(1) pledge (& updated kernel diff)
On Sun, Dec 30, 2018 at 10:58:58AM +0100, Landry Breuil wrote:
> On Sat, Dec 29, 2018 at 09:30:22AM +0100, Sebastien Marie wrote:
> > On Fri, Dec 28, 2018 at 09:41:06PM +0100, Landry Breuil wrote:
>
> > I think you pledged too early.
> >
> > "dns unix" are here for XOpenDisplay(), but if the display is remote,
> > "inet" could be needed too. Even if Xv isn't possible on remote display,
> > XOpenDisplay() will be killed by pledge.
> >
> > So ideally, pledge(2) should occurs later.
>
> Yup, after some discussions, moving the pledge calls after setup()
> (which does the file & xv opening) allows to only need stdio in the -i
> case, and 'stdio rpath video' in the others. i thought rpath could be
> dropped but in my testing with video -vvv -O i had a crash upon some
> keys where x tried to open /usr/X11R6/share/X11/locale/locale.alias..
libX11 tentacles :) I think any program using X11 needs rpath.
> in addition we can also pledge the video -q case which needs
> stdio/rpath/wpath/video.
The promises looks more sanely now.
For me, there is still initialization stuff in dev_check_caps(), a
function used to query capabilities of the device (if the device is able
to record video, and how [via read-write or via streaming (mmap)]).
As it is the first function using ioctl(2) on the device, it opens
the descriptor device (using O_RDWR). It is why we need rpath+wpath
here. But such refactoring is outside the scope of the diff. No problem
with that.
Thanks. I will review the kernel side soon.
> Index: video.c
> ===
> RCS file: /cvs/xenocara/app/video/video.c,v
> retrieving revision 1.25
> diff -u -r1.25 video.c
> --- video.c 9 Apr 2018 18:16:44 - 1.25
> +++ video.c 30 Dec 2018 09:39:27 -
> @@ -1961,6 +1961,8 @@
> argv += optind;
>
> if (vid.mode & M_QUERY) {
> + if (pledge("stdio rpath wpath video", NULL) == -1)
> + err(1, "pledge");
> dev_dump_query(&vid);
> cleanup(&vid, 0);
> }
> @@ -1970,6 +1972,14 @@
>
> if (!setup(&vid))
> cleanup(&vid, 1);
> +
> + if (vid.mode & M_IN_FILE) {
> + if (pledge("stdio", NULL) == -1)
> + err(1, "pledge");
> + } else {
> + if (pledge("stdio rpath video", NULL) == -1)
> + err(1, "pledge");
> + }
>
> if (!stream(&vid))
> cleanup(&vid, 1);
--
Sebastien Marie
Re: video(1) pledge (& updated kernel diff)
On Sun, Dec 30, 2018 at 10:58:58AM +0100, Landry Breuil wrote:
> On Sat, Dec 29, 2018 at 09:30:22AM +0100, Sebastien Marie wrote:
> > On Fri, Dec 28, 2018 at 09:41:06PM +0100, Landry Breuil wrote:
> >
> > I would separate the addition of pledge(2) and unrelated fixes.
>
> Right, two separated diffs attached for this.
>
the err->errs diff to avoid shadowing is ok semarie@
mherrb@, any objections to it ?
Thanks
--
Sebastien Marie
> Index: video.c
> ===
> RCS file: /cvs/xenocara/app/video/video.c,v
> retrieving revision 1.25
> diff -u -r1.25 video.c
> --- video.c 9 Apr 2018 18:16:44 - 1.25
> +++ video.c 30 Dec 2018 09:39:21 -
> @@ -1854,7 +1854,7 @@
> struct xdsp *x = &vid.xdsp;
> const char *errstr;
> size_t len;
> - int ch, err = 0;
> + int ch, errs = 0;
>
> bzero(&vid, sizeof(struct video));
>
> @@ -1872,21 +1872,21 @@
> x->cur_adap = strtonum(optarg, 0, 4, &errstr);
> if (errstr != NULL) {
> warnx("Xv adaptor '%s' is %s", optarg, errstr);
> - err++;
> + errs++;
> }
> break;
> case 'e':
> vid.enc = find_enc(optarg);
> if (vid.enc >= ENC_LAST) {
> warnx("encoding '%s' is invalid", optarg);
> - err++;
> + errs++;
> }
> break;
> case 'f':
> len = strlcpy(d->path, optarg, sizeof(d->path));
> if (len >= sizeof(d->path)) {
> warnx("file path is too long: %s", optarg);
> - err++;
> + errs++;
> }
> break;
> case 'g':
> @@ -1894,8 +1894,8 @@
> break;
> case 'i':
> if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
> - warnx("only one input or ouput file allowed");
> - err++;
> + warnx("only one input or output file allowed");
> + errs++;
> } else {
> vid.mode = (vid.mode & ~M_IN_DEV) | M_IN_FILE;
> vid.mmap_on = 0; /* mmap mode does not work for
> files */
> @@ -1904,15 +1904,15 @@
> if (len >= sizeof(vid.iofile)) {
> warnx("input path is too long: %s",
> optarg);
> - err++;
> + errs++;
> }
> }
> break;
> case 'o':
> case 'O':
> if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
> - warnx("only one input or ouput file allowed");
> - err++;
> + warnx("only one input or output file allowed");
> + errs++;
> } else {
> vid.mode |= M_OUT_FILE;
> if (ch != 'O')
> @@ -1922,7 +1922,7 @@
> if (len >= sizeof(vid.iofile)) {
> warnx("output path is too long: %s",
> optarg);
> - err++;
> + errs++;
> }
> }
> break;
> @@ -1937,7 +1937,7 @@
> vid.fps = strtonum(optarg, 1, 100, &errstr);
> if (errstr != NULL) {
> warnx("frame rate '%s' is %s", optarg, errstr);
> - err++;
> + errs++;
> }
> break;
> case 's':
> @@ -1947,13 +1947,13 @@
> vid.verbose++;
> break;
> default:
> - err++;
> + errs++;
> break;
> }
> - if (err > 0)
> + if (errs > 0)
> break;
> }
> - if (err > 0) {
> + if (errs > 0) {
> usage();
> cleanup(&vid, 1);
> }
Re: video(1) pledge (& updated kernel diff)
On Sat, Dec 29, 2018 at 09:30:22AM +0100, Sebastien Marie wrote:
> On Fri, Dec 28, 2018 at 09:41:06PM +0100, Landry Breuil wrote:
> > Hi,
> >
> > so i've updated my 'video' class for pledge and also did an initial
> > naive pledging of xenocara/app/video:
>
> just a small note: video(1) is the sole customer for ioctl(VIDIOC_*) in
> base. other potential customers are in ports.
>
> personally, with such promise, I would be interested in pledging a SIP
> program (like telephony/baresip, but I didn't check if this program is
> designed well enough for be pledgeable). I will look at it in order to
> have a third program to check the relevance of the ioctls.
>
> > as for the video(1) diff:
> > - fixed a typo
> > - renamed err to errs to avoid shadowing err()
>
> I would separate the addition of pledge(2) and unrelated fixes.
Right, two separated diffs attached for this.
> I think you pledged too early.
>
> "dns unix" are here for XOpenDisplay(), but if the display is remote,
> "inet" could be needed too. Even if Xv isn't possible on remote display,
> XOpenDisplay() will be killed by pledge.
>
> So ideally, pledge(2) should occurs later.
Yup, after some discussions, moving the pledge calls after setup()
(which does the file & xv opening) allows to only need stdio in the -i
case, and 'stdio rpath video' in the others. i thought rpath could be
dropped but in my testing with video -vvv -O i had a crash upon some
keys where x tried to open /usr/X11R6/share/X11/locale/locale.alias..
in addition we can also pledge the video -q case which needs
stdio/rpath/wpath/video.
> > +#define PLEDGE_VIDEO 0x00200800ULL /* video ioctls */
>
> I suspect a copy/paste error: PLEDGE_VIDEO contains 2 bits sets (there is a
> '8' after the '2').
> and space vs tab between the name and the value.
Yes that was a typo, i think when i originally started this the next
available define was 8000ULL :)
Landry
Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.25
diff -u -r1.25 video.c
--- video.c 9 Apr 2018 18:16:44 - 1.25
+++ video.c 30 Dec 2018 09:39:21 -
@@ -1854,7 +1854,7 @@
struct xdsp *x = &vid.xdsp;
const char *errstr;
size_t len;
- int ch, err = 0;
+ int ch, errs = 0;
bzero(&vid, sizeof(struct video));
@@ -1872,21 +1872,21 @@
x->cur_adap = strtonum(optarg, 0, 4, &errstr);
if (errstr != NULL) {
warnx("Xv adaptor '%s' is %s", optarg, errstr);
- err++;
+ errs++;
}
break;
case 'e':
vid.enc = find_enc(optarg);
if (vid.enc >= ENC_LAST) {
warnx("encoding '%s' is invalid", optarg);
- err++;
+ errs++;
}
break;
case 'f':
len = strlcpy(d->path, optarg, sizeof(d->path));
if (len >= sizeof(d->path)) {
warnx("file path is too long: %s", optarg);
- err++;
+ errs++;
}
break;
case 'g':
@@ -1894,8 +1894,8 @@
break;
case 'i':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
- warnx("only one input or ouput file allowed");
- err++;
+ warnx("only one input or output file allowed");
+ errs++;
} else {
vid.mode = (vid.mode & ~M_IN_DEV) | M_IN_FILE;
vid.mmap_on = 0; /* mmap mode does not work for
files */
@@ -1904,15 +1904,15 @@
if (len >= sizeof(vid.iofile)) {
warnx("input path is too long: %s",
optarg);
- err++;
+ errs++;
}
}
break;
case 'o':
case 'O':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
- warnx("only one input or ouput file allowed");
- err++;
+ warnx("only one input or output file allowed");
+ errs++;
} else {
vid.mode |= M_OUT_FILE;
if (ch != 'O')
@@ -1922,7 +1
Re: video(1) pledge (& updated kernel diff)
On Fri, Dec 28, 2018 at 09:41:06PM +0100, Landry Breuil wrote:
> Hi,
>
> so i've updated my 'video' class for pledge and also did an initial
> naive pledging of xenocara/app/video:
just a small note: video(1) is the sole customer for ioctl(VIDIOC_*) in
base. other potential customers are in ports.
personally, with such promise, I would be interested in pledging a SIP
program (like telephony/baresip, but I didn't check if this program is
designed well enough for be pledgeable). I will look at it in order to
have a third program to check the relevance of the ioctls.
> as for the video(1) diff:
> - fixed a typo
> - renamed err to errs to avoid shadowing err()
I would separate the addition of pledge(2) and unrelated fixes.
> and i've identified 4 main modes:
> - -o needs to write to the fs (so wpath cpath) and read from the device
> (so video)
> - -O needs the same subset and dns unix in addition, as it talks to Xv
> - -i only needs rpath & dns/unix for xv, no access to the video device
> needed
> - no option needs dns unix for xv, video for ioctls, and wpath as the
> video device is open with O_RDWR.
>
> tested those 4 modes without aborts so far, video -q exits before
> reaching pledge. Not sure if those pledge calls are at the best spot,
> but that's a start.
>
> I know video(1) sucks and doesnt work directly with Xv with most modern
> builtin devices on laptops as they often dont support the few encodings
> supported by video(1) for Xv, but with my old-school cameras lying
> around it works fine.
I have old enough stuff to playing with video(1), so I will test it :)
> Index: video.c
> ===
> RCS file: /cvs/xenocara/app/video/video.c,v
> retrieving revision 1.25
> diff -u -r1.25 video.c
> --- video.c 9 Apr 2018 18:16:44 - 1.25
> +++ video.c 28 Dec 2018 20:22:36 -
> @@ -1963,6 +1963,22 @@
> if (vid.mode & M_QUERY) {
> dev_dump_query(&vid);
> cleanup(&vid, 0);
> + }
> +
> + if (vid.mode & M_OUT_FILE) {
> + if(vid.mode & M_OUT_XV) {
> + if (pledge("stdio rpath wpath cpath dns unix video",
> NULL) == -1)
> + err(1, "pledge");
> + } else {
> + if (pledge("stdio rpath wpath cpath video", NULL) == -1)
> + err(1, "pledge");
> + }
> + } else if (vid.mode & M_IN_FILE) {
> + if (pledge("stdio rpath dns unix", NULL) == -1)
> + err(1, "pledge");
> + } else {
> + if (pledge("stdio rpath wpath dns unix video", NULL) == -1)
> + err(1, "pledge");
> }
>
> if (vid.fps == 0)
I think you pledged too early.
"dns unix" are here for XOpenDisplay(), but if the display is remote,
"inet" could be needed too. Even if Xv isn't possible on remote display,
XOpenDisplay() will be killed by pledge.
So ideally, pledge(2) should occurs later.
> Index: kern/kern_pledge.c
> ===
> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.245
> diff -u -r1.245 kern_pledge.c
> --- kern/kern_pledge.c17 Nov 2018 23:10:08 - 1.245
> +++ kern/kern_pledge.c28 Dec 2018 20:23:02 -
> @@ -43,6 +43,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -396,6 +397,7 @@
> { "tty",PLEDGE_TTY },
> { "unix", PLEDGE_UNIX },
> { "unveil", PLEDGE_UNVEIL },
> + { "video", PLEDGE_VIDEO },
> { "vminfo", PLEDGE_VMINFO },
> { "vmm",PLEDGE_VMM },
> { "wpath", PLEDGE_WPATH },
> @@ -1158,6 +1160,34 @@
> break;
> }
> }
> +
> + if ((p->p_p->ps_pledge & PLEDGE_VIDEO)) {
> + switch (com) {
> + case VIDIOC_QUERYCAP:
> + case VIDIOC_TRY_FMT:
> + case VIDIOC_ENUM_FMT:
> + case VIDIOC_S_FMT:
> + case VIDIOC_QUERYCTRL:
> + case VIDIOC_G_CTRL:
> + case VIDIOC_S_CTRL:
> + case VIDIOC_G_PARM:
> + case VIDIOC_S_PARM:
> + case VIDIOC_REQBUFS:
> + case VIDIOC_QBUF:
> + case VIDIOC_DQBUF:
> + case VIDIOC_QUERYBUF:
> + case VIDIOC_STREAMON:
> + case VIDIOC_STREAMOFF:
> + case VIDIOC_ENUM_FRAMESIZES:
> + case VIDIOC_ENUM_FRAMEINTERVALS:
I didn't check them for now. I will do it later.
Please note that I will take in consideration all ioctls provided by
video(4), and not only the one used by video(1) or firefox. The purpose
is to have usable "video" promise, without been widely opened neither
too restricted. It is why we should considere several programs at once
when designing the promise, and l
video(1) pledge (& updated kernel diff)
Hi,
so i've updated my 'video' class for pledge and also did an initial
naive pledging of xenocara/app/video:
the kernel diff is the same subset as the diff from some months ago,
except that it adds:
- VIDIOC_QUERYCTRL/VIDIOC_G_CTRL/VIDIOC_S_CTRL: those 3 are used by
video(1) to set gamma/contrast/etc controls on the device on the fly
(via A/a, B/b, C/c - etc when running)
- VIDIOC_TRY_FMT, used by
https://searchfox.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_capture/linux/device_info_linux.cc#418
since https://bugzilla.mozilla.org/show_bug.cgi?id=1376873 (yes, i
checked with ktrace, all the other ioctls are still used)
with that kernel diff, i'm able to record video in firefox with the
video pledge added to the main process.
as for the video(1) diff:
- fixed a typo
- renamed err to errs to avoid shadowing err()
and i've identified 4 main modes:
- -o needs to write to the fs (so wpath cpath) and read from the device
(so video)
- -O needs the same subset and dns unix in addition, as it talks to Xv
- -i only needs rpath & dns/unix for xv, no access to the video device
needed
- no option needs dns unix for xv, video for ioctls, and wpath as the
video device is open with O_RDWR.
tested those 4 modes without aborts so far, video -q exits before
reaching pledge. Not sure if those pledge calls are at the best spot,
but that's a start.
I know video(1) sucks and doesnt work directly with Xv with most modern
builtin devices on laptops as they often dont support the few encodings
supported by video(1) for Xv, but with my old-school cameras lying
around it works fine.
Landry
? ktrace.out
? video
? video.d
Index: video.c
===
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.25
diff -u -r1.25 video.c
--- video.c 9 Apr 2018 18:16:44 - 1.25
+++ video.c 28 Dec 2018 20:22:36 -
@@ -1854,7 +1854,7 @@
struct xdsp *x = &vid.xdsp;
const char *errstr;
size_t len;
- int ch, err = 0;
+ int ch, errs = 0;
bzero(&vid, sizeof(struct video));
@@ -1872,21 +1872,21 @@
x->cur_adap = strtonum(optarg, 0, 4, &errstr);
if (errstr != NULL) {
warnx("Xv adaptor '%s' is %s", optarg, errstr);
- err++;
+ errs++;
}
break;
case 'e':
vid.enc = find_enc(optarg);
if (vid.enc >= ENC_LAST) {
warnx("encoding '%s' is invalid", optarg);
- err++;
+ errs++;
}
break;
case 'f':
len = strlcpy(d->path, optarg, sizeof(d->path));
if (len >= sizeof(d->path)) {
warnx("file path is too long: %s", optarg);
- err++;
+ errs++;
}
break;
case 'g':
@@ -1894,8 +1894,8 @@
break;
case 'i':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
- warnx("only one input or ouput file allowed");
- err++;
+ warnx("only one input or output file allowed");
+ errs++;
} else {
vid.mode = (vid.mode & ~M_IN_DEV) | M_IN_FILE;
vid.mmap_on = 0; /* mmap mode does not work for
files */
@@ -1904,15 +1904,15 @@
if (len >= sizeof(vid.iofile)) {
warnx("input path is too long: %s",
optarg);
- err++;
+ errs++;
}
}
break;
case 'o':
case 'O':
if (vid.mode & (M_IN_FILE | M_OUT_FILE)) {
- warnx("only one input or ouput file allowed");
- err++;
+ warnx("only one input or output file allowed");
+ errs++;
} else {
vid.mode |= M_OUT_FILE;
if (ch != 'O')
@@ -1922,7 +1922,7 @@
if (len >= sizeof(vid.iofile)) {
warnx("output path is too long: %s",
optarg);
- err++;
+
