Date: Wed, 12 Jun 2019 23:02:16 -0600 From: Warner Losh <imp@bsdimp.com> To: Fuqian Huang <huangfq.daxian@gmail.com> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: Dev:Ciss: A kernel address leakage in sys/dev/ciss/ciss.c Message-ID: <CANCZdfqEnV0gLF5mG19cxMaAoU%2Bt8xhHx4FoEsC6ScKhpCZ6Og@mail.gmail.com> In-Reply-To: <CABXRUiQ30KkP0fjYVrJCaLgCM4uPOOS1RShF6p9TDd58ZDhF3w@mail.gmail.com> References: <CABXRUiTJAxRWdTsBP5K-5axAV-EZO0ddxhStwWGDDWoi7Hwsww@mail.gmail.com> <CANCZdfoJH4y4aOZ459rarUX7L6Fd==24YGHPidEdEMrbOuAbhw@mail.gmail.com> <CABXRUiQ30KkP0fjYVrJCaLgCM4uPOOS1RShF6p9TDd58ZDhF3w@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Because CISS_FLAG_NOTIFY_OK will be clear in sc->ciss_flags. It's only set
when we have the notifier armed, which is only armed when we have outanding
commands in the CISS hardware. Since we know that all the outstanding
requests have completed before the close, it should be clear. But again,
this only gets called at the end of ciss_attach (on the error path before
the notifier is setup) or in ciss_detach (which is only called when the
ciss driver is unloaded with the device attached, but unused, or if the
user has administratively put the interface down). So even if I've read
things and a notifier is running, the only way to provoke it is to do a
root-only operation. Disclosing kernel addresses to root doesn't seem like
a big deal, especially since the dmesg can be secured from non-root users.
This is why I've said this is more theoretical than actual. You need root
permissions to provoke it, and then root permissions to read the dmesg.
I'll commit a #if 0 out of an abundance of caution, but I'm having trouble
seeing how non-root users could provoke it.
Then again, the ciss driver is for older hardware that is somewhat rare
these days.
Thanks for the report...
Warner
On Wed, Jun 12, 2019 at 8:54 PM Fuqian Huang <huangfq.daxian@gmail.com>
wrote:
> But, why there will be no commands that are printed?
> 'cr' is get from ciss_get_request and 'cr->cr_data' is the result of
> malloc in ciss_notify_abort, and they are freed after the 'out' label.
> At the printing point, some address has been printed out.
> I know what you mean that this only happens when detaching the device.
> But it seems that some address is printed out before the free
> operation, and is it necessary to print the address?
>
> Warner Losh <imp@bsdimp.com> =E6=96=BC 2019=E5=B9=B46=E6=9C=8813=E6=97=A5=
=E9=80=B1=E5=9B=9B =E4=B8=8A=E5=8D=885:51=E5=AF=AB=E9=81=93=EF=BC=9A
> >
> >
> >
> > On Wed, Jun 12, 2019 at 7:02 AM Fuqian Huang <huangfq.daxian@gmail.com>
> wrote:
> >>
> >> In freebsd/sys/dev/ciss/ciss.c, function ciss_print_request will dump
> >> the address of a kernel object cr to user space. Each time when a
> >> device is detached, it will call
> >> ciss_free->ciss_notify_abort->ciss_print_request, and this finally
> >> dump a kernel address to user space.
> >
> >
> > This is, at best, a theoretical concern. ciss_detach isn't called excep=
t
> when detaching the device. This only happens if you are unloading the
> module or using devctl to detach it. Second, the bit you chopped out of
> ciss_detach ensure that the controller isn't open. Close is only called
> when there's no pending requests from geom to the device, and we get call=
ed
> for the LAST close, meaning nothing else has it open. This means there wi=
ll
> be no commands to abort when ciss_notify_abort() is called. Since there's
> no commands to abort, there will be no commands that are printed, so no
> user address will be disclosed.
> >
> > Having said that, do you have a test case that can trigger this? It
> would be most unexpected indeed...
> >
> > Warner
> >
> >>
> >> static int
> >> ciss_detach(device_t dev)
> >> {
> >> struct ciss_softc *sc =3D device_get_softc(dev);
> >> ...
> >> ciss_free(sc);
> >> return (0);
> >> }
> >>
> >> static void
> >> ciss_free(struct ciss_softc *sc)
> >> {
> >> ...
> >> -> ciss_notify_abort(sc);
> >> ...
> >> }
> >>
> >> static int
> >> ciss_notify_abort(struct ciss_softc *sc)
> >> {
> >> struct ciss_request *cr;
> >> ...
> >> if ((error =3D ciss_get_request(sc, &cr))
> >> goto out;
> >> ...
> >> -> ciss_print_request(cr);
> >> ...
> >> }
> >>
> >> static void
> >> ciss_print_request(struct ciss_request *cr)
> >> {
> >> struct ciss_softc *sc;
> >> ...
> >> sc =3D cr->cr_sc;
> >> ...
> >> -> ciss_printf(sc, "REQUEST @ %p\n", cr);
> >> ciss_printf(sc, " data %p/%d tag %d flags %b\n",
> >> cr->cr_data, cr->cr_length, cr->cr_tag, cr->cr_flags,
> >> "\20\1mapped\2sleep\3poll\4dataout\5datain\n");
> >> }
> >> _______________________________________________
> >> freebsd-hackers@freebsd.org mailing list
> >> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> >> To unsubscribe, send any mail to "
> freebsd-hackers-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfqEnV0gLF5mG19cxMaAoU%2Bt8xhHx4FoEsC6ScKhpCZ6Og>