Re: [rfc] /dev/devstat permissions patch

On Thu, Mar 20, 2014 at 7:05 AM, John Baldwin <jhb_at_freebsd.org> wrote:
> On Tuesday, March 18, 2014 3:29:32 pm Maksim Yevmenkin wrote:
>> hello,
>>
>> would anyone object to the following patch?
>
> I think this is fine.
>
> While you are at it, can you test this patch to remove D_NEEDGIANT?
>
> Index: subr_devstat.c
> ===================================================================
> --- subr_devstat.c      (revision 263302)
> +++ subr_devstat.c      (working copy)
> _at__at_ -460,7 +460,6 _at__at_ static d_mmap_t devstat_mmap;
>
>  static struct cdevsw devstat_cdevsw = {
>         .d_version =    D_VERSION,
> -       .d_flags =      D_NEEDGIANT,
>         .d_mmap =       devstat_mmap,
>         .d_name =       "devstat",
>  };
> _at__at_ -482,13 +481,16 _at__at_ devstat_mmap(struct cdev *dev, vm_ooffset_t offset
>
>         if (nprot != VM_PROT_READ)
>                 return (-1);
> +       mtx_lock(&devstat_mutex);
>         TAILQ_FOREACH(spp, &pagelist, list) {
>                 if (offset == 0) {
>                         *paddr = vtophys(spp->stat);
> +                       mtx_unlock(&devstat_mutex);
>                         return (0);
>                 }
>                 offset -= PAGE_SIZE;
>         }
> +       mtx_unlock(&devstat_mutex);
>         return (-1);
>  }

seems to work fine for me. so, i guess, i will commit combined patch, then

==

Index: subr_devstat.c
===================================================================
--- subr_devstat.c (revision 3427)
+++ subr_devstat.c (working copy)
_at__at_ -462,7 +462,6 _at__at_

 static struct cdevsw devstat_cdevsw = {
  .d_version = D_VERSION,
- .d_flags = D_NEEDGIANT,
  .d_mmap = devstat_mmap,
  .d_name = "devstat",
 };
_at__at_ -484,13 +483,16 _at__at_

  if (nprot != VM_PROT_READ)
  return (-1);
+ mtx_lock(&devstat_mutex);
  TAILQ_FOREACH(spp, &pagelist, list) {
  if (offset == 0) {
  *paddr = vtophys(spp->stat);
+ mtx_unlock(&devstat_mutex);
  return (0);
  }
  offset -= PAGE_SIZE;
  }
+ mtx_unlock(&devstat_mutex);
  return (-1);
 }

_at__at_ -505,7 +507,7 _at__at_
  mtx_assert(&devstat_mutex, MA_NOTOWNED);
  if (!once) {
  make_dev_credf(MAKEDEV_ETERNAL | MAKEDEV_CHECKNAME,
-    &devstat_cdevsw, 0, NULL, UID_ROOT, GID_WHEEL, 0400,
+    &devstat_cdevsw, 0, NULL, UID_ROOT, GID_WHEEL, 0444,
     DEVSTAT_DEVICE_NAME);
  once = 1;
  }

==


thanks
max