On Sat, Oct 24, 2020 at 11:34:07AM -0600, Theo de Raadt wrote:
> Marcus Glocker <[email protected]> wrote:
>
> > On Sun, Sep 27, 2020 at 05:57:51PM +0100, Laurence Tratt wrote:
> >
> > > On Sun, Sep 13, 2020 at 09:23:36AM +0100, Laurence Tratt wrote:
> > >
> > > > Since I recently opened my big fat mouth and suggested that
> > > > "kern.video.record" (analogous to kern.audio.record) might be a good
> > > > idea, I
> > > > decided to put together a quick prototype (heavily based on the
> > > > kern.audio.record code). This at least roughly works for me but raises
> > > > some
> > > > questions such as:
> > > >
> > > > * Is uvideo the only driver that can capture video? [I imagine not,
> > > > but I
> > > > don't really know.]
> >
> > 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:08:28 -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)
@@ -302,6 +311,8 @@ videoioctl(dev_t dev, u_long cmd, caddr_
sc->sc_frames_ready--;
break;
case VIDIOC_STREAMON:
+ if (!video_record_enable)
+ return (EPERM);
if (sc->hw_if->streamon)
error = (sc->hw_if->streamon)(sc->hw_hdl,
(int)*data);
