On Sat, Oct 24, 2020 at 01:17:05PM -0600, Theo de Raadt wrote:
> Seems better.
>
> Toggling the behaviour while video occurs should be tested, to make
> sure it matches expectations.
Right, works already for read(2) but for mmap(2) we need to move the
check to the VIDIOC_DQBUF ioctl(2) then.
I can bake a fresh, full diff for further testing.
> > > > utvfu(4) comes in mind.
> > >
> > > And therefore, it makes no sense the diff contains change to uvideo(4)
> > > only. I know video(4) is a thin shim, but the blocking logic isn't
> > > correctly placed.
> >
> > I agree.
> >
> > Why don't we simply deny turning on the cams video stream when
> > kern.video.record=0 in video(1)? See diff.
> >
> > imac:~ [hacki] $ doas video
> > video: ioctl VIDIOC_STREAMON: Operation not permitted
> >
> > > One more comment. If the bcopy's are skipped, what data is in those
> > > buffers? Are they zero'd freshly, or allocated with a M_ZERO type
> > > allocation, or what's the story? Or if one toggles the control do
> > > the buffers continue to show old records rather than 'black'?
> >
> > The video buffers aren't initialized ever in uvideo(4).
> > But with the above suggestion, we don't need to worry about the buffers
> > anymore, they simply won't get passed (nor even filled).
Index: video.c
===================================================================
RCS file: /cvs/src/sys/dev/video.c,v
retrieving revision 1.44
diff -u -p -u -p -r1.44 video.c
--- video.c 16 May 2020 10:47:22 -0000 1.44
+++ video.c 24 Oct 2020 19:36:17 -0000
@@ -40,6 +40,12 @@
#define DPRINTF(x)
#endif
+/*
+ * Global flag to control if audio recording is enabled when the
+ * mixerctl setting is record.enable=sysctl
+ */
+int video_record_enable = 0;
+
struct video_softc {
struct device dev;
void *hw_hdl; /* hardware driver handle */
@@ -159,6 +165,9 @@ videoread(dev_t dev, struct uio *uio, in
int unit, error;
size_t size;
+ if (!video_record_enable)
+ return (EPERM);
+
unit = VIDEOUNIT(dev);
if (unit >= video_cd.cd_ndevs ||
(sc = video_cd.cd_devs[unit]) == NULL)
@@ -290,6 +299,8 @@ videoioctl(dev_t dev, u_long cmd, caddr_
(struct v4l2_buffer *)data);
break;
case VIDIOC_DQBUF:
+ if (!video_record_enable)
+ return (EPERM);
if (!sc->hw_if->dqbuf)
break;
/* should have called mmap() before now */
