Date: Mon, 16 Jun 2003 14:28:44 -0700 From: John-Mark Gurney <gurney_j@efn.org> To: Robert Watson <rwatson@freebsd.org> Cc: arch@freebsd.org Subject: Re: make /dev/pci really readable Message-ID: <20030616212844.GU73854@funkthat.com> In-Reply-To: <Pine.NEB.3.96L.1030616155956.8726C-100000@fledge.watson.org> References: <20030616184204.GL73854@funkthat.com> <Pine.NEB.3.96L.1030616155956.8726C-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Robert Watson wrote this message on Mon, Jun 16, 2003 at 16:12 -0400:
> I guess I found the question unclear -- you didn't quite specify what the
> change you were making was...
Yes, sorry, that is true. (and originally I was thinking of making
-r publicly available too.)
> > > odd use of useracc(), printf(), etc, in the ioctl code. I suspect this
> >
> > well, do you mean odd use of printf as in providing diagnostics to catch
> > mismatched userland/kernel?
>
> Spamming the console with messages can have debilitating effects on the
> operation of the system if performed by unprivileged users... I.e., if
> you're using a serial console.
Very true, I forget that there is always the DoS attack on logs too.
Ok, I'll look at either adding return values or just passing EINVAL
back. Also, since pciconf is currently the only consumer, most of the
checks really are only for debugging purposes.
> > for useracc, it checks to make sure that various pointers passed to it
> > are either readable or writable. I don't see this as odd. Or is there
> > another better method of checking user data when accessing user space
> > buffers?
>
> Generally, calls to useracc() are redundant with the existing checks in
> our copyin/out routines, or are signs that the proper routines aren't
> being used. All of the fuword/suword/copyin/copyout/uio routines already
> perform any necessary checks; manual checking is race-prone in a
> multi-threaded smp environment regardless. The (cio==NULL) test is also
Ahh, k, I did not know this, and nothing like this is mentioned about
this in the manpage. I'll remove the useracc calls since they are
unecessary.
> redundant. It looks like (although I haven't tried), user processes can
> also cause the kernel to allocate unlimited amounts of kernel memory,
> which is another bit we probably need to tighten down.
Hmmm. I'll take a look at this, but I did think the cio == NULL test was
redundant, but "safe".. :)
> > other than a minor bug that could hit if there was more pci_devinfo's in
> > the list than pci_numdevs (which should never happen, but will prevent a
> > NULL deref), I didn't see anything wrong with -l.
>
> Allowing arbitrary users to panic the system is bad :-).
>
> > > code needs some fairly thorough review and cleanup before we should reduce
> > > the level of privilege required to use the device (note that we make it
> > > world readable by default, so changes in the semantics of read permissions
> >
> > > will affect all users in the system). Could you do that cleanup in the
> > > first pass, then revisit the permissions change?
> >
> > sure, no problem.
>
> Great. I think that there's nothing with the idea of loosening up the
> restrictions here; one question I do have to wonder about in the long term
> is whether this is information that should be exported using one of the
> existing device general-purpose device configuration monitoring
> interfaces, such as the sysctls used to support devinfo(8) in some more
> generic form.
Well, if this information can/will be exported via another mechanism,
we might as well remove this interface. devinfo can support all the
information the pciconf -l exports right now. It just hasn't been taught
about /usr/shar/emisc/pci_vendors yet.
Though I don't know of another interface that would let us read/write
pci config registers safely though.
--
John-Mark Gurney Voice: +1 415 225 5579
"All that I will do, has been done, All that I have, has not."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030616212844.GU73854>