Seems better.

Toggling the behaviour while video occurs should be tested, to make
sure it matches expectations.


Marcus Glocker <[email protected]> wrote:

> 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);

Reply via email to