Re: [PATCH]media:dvb: add compat_ioctl def for dmx_dvr

Re: [PATCH]media:dvb: add compat_ioctl def for dmx_dvr

[Arnd Bergmann]

On Mon, Jan 18, 2021 at 7:21 AM root jason <jason.root.w@gmail.com> wrote:
> From: jason.wang <jason.root.w@gmail.com>
>
> add compat_ioctl define for dmx_dvr to handle ioctl when CONFIG_COMPAT is enable.
>
> Signed-off-by: .jason.wang <jason.root.w@gmail.com>
> ---
>  drivers/media/dvb-core/dmxdev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> index f14a872d1268..4a9e027de827 100644
> --- a/drivers/media/dvb-core/dmxdev.c
> +++ b/drivers/media/dvb-core/dmxdev.c
> @@ -1393,6 +1393,7 @@ static const struct file_operations dvb_dvr_fops = {
>         .read = dvb_dvr_read,
>         .write = dvb_dvr_write,
>         .unlocked_ioctl = dvb_dvr_ioctl,
> +       .compat_ioctl = dvb_dvr_ioctl,
>         .open = dvb_dvr_open,
>         .release = dvb_dvr_release,
>         .poll = dvb_dvr_poll,

This is correct for DMX_SET_BUFFER_SIZE, which takes an integer
argument, but not strictly correct for the other ones that take a pointer
argument and need a compat_ptr() conversion.

You could do it by either passing both the 'unsigned long arg'
and the 'void __user *argp' pointer to dvb_usercopy(), with the
pointer coming from compat_ptr() in case of compat, or you
add something like

         if (in_compat_syscall())
                   arg = compat_ptr(unsigned long arg);

in the function itself. I checked the DVB ioctls to make sure that
no other ioctl commands need any special handling, and found
that DMX_SET_BUFFER_SIZE is the only one.

          Arnd

Re: [PATCH]media:dvb: add compat_ioctl def for dmx_dvr

[root jason]

Arnd Bergmann <arnd@kernel.org> 于2021年1月18日周一 下午3:38写道：
>
> On Mon, Jan 18, 2021 at 7:21 AM root jason <jason.root.w@gmail.com> wrote:
> > From: jason.wang <jason.root.w@gmail.com>
> >
> > add compat_ioctl define for dmx_dvr to handle ioctl when CONFIG_COMPAT is enable.
> >
> > Signed-off-by: .jason.wang <jason.root.w@gmail.com>
> > ---
> >  drivers/media/dvb-core/dmxdev.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
> > index f14a872d1268..4a9e027de827 100644
> > --- a/drivers/media/dvb-core/dmxdev.c
> > +++ b/drivers/media/dvb-core/dmxdev.c
> > @@ -1393,6 +1393,7 @@ static const struct file_operations dvb_dvr_fops = {
> >         .read = dvb_dvr_read,
> >         .write = dvb_dvr_write,
> >         .unlocked_ioctl = dvb_dvr_ioctl,
> > +       .compat_ioctl = dvb_dvr_ioctl,
> >         .open = dvb_dvr_open,
> >         .release = dvb_dvr_release,
> >         .poll = dvb_dvr_poll,
>
> This is correct for DMX_SET_BUFFER_SIZE, which takes an integer
> argument, but not strictly correct for the other ones that take a pointer
> argument and need a compat_ptr() conversion.
>
> You could do it by either passing both the 'unsigned long arg'
> and the 'void __user *argp' pointer to dvb_usercopy(), with the
> pointer coming from compat_ptr() in case of compat, or you
> add something like
>
>          if (in_compat_syscall())
>                    arg = compat_ptr(unsigned long arg);
>
> in the function itself. I checked the DVB ioctls to make sure that
> no other ioctl commands need any special handling, and found
> that DMX_SET_BUFFER_SIZE is the only one.
>
>           Arnd

hi Arnd,
     thank you for your quick comment.

     I upload this change is because if not compt_ioctl dvr_ioctl can
not be invoked
     I checked my local kernel code about compt_ioctl call flow.
     I figure out that my local kernel version is 5.4.70.
     if compat_ioctl is not defined, ioctl cmd need to add into the
array(ioctl_pointer),
     otherwise, unlocked_ioctl will not be called.

     and I check latest kernel version. the logic about this part is
changed.  no need to
     add ioctl cmd into that array.
     for the latest kernel, it should be ok.

    anyway, .thank you for your reply.

jason.wang