On 13 Mar 2015, at 12:53, Christos Zoulas <chris...@zoulas.com> wrote:
> On Mar 13, 11:19am, hann...@eis.cs.tu-bs.de ("J. Hannken-Illjes") wrote: > -- Subject: Re: DoS attack against TCP services > > | The mutex involved is the sme_mtx protecting the struct sysmon_envsys, so > | our problem doesn't come from missing POLL. > > That's what I thought. > > | We already have it. If I understand sysmon right, it is already based on > | workqueues (the ciss0 thread here): > | > | The workqueue updates sensors every sme->sme_events_timeout seconds, defaul= > | t > | is 30 seconds. Workqueue items get enqueued from a callout. > | > | Both running a workqueue item and processing the callout locks the > | same mutex sme->sme_mtx. > | > | For this to work the workqueue must complete before the callout fires: > | > | sme->sme_nsensors * ccb->ccb_xs->timeout < sme->sme_events_timeout > | > | In our ciss case we could set: > | > | sc->sc_sme->sme_events_timeout =3D 30 > | > | ccb->ccb_xs->timeout=3D 20 / sc->maxunits > | > | to become safe. > | > | Hope I got this right so far ... > > Yes, you do. We could decrease the timeout for probing, but that might > lead to unsuccessful sensor reads. Even then perhaps there is a place > to have a special mode for sysmon to use a separate thread for reading > the sensors of a particular driver, or a way to change the sysmon period > to be longer. This would be simple, changing dev/ic/ciss.c like: sc->sc_sme->sme_name = device_xname(sc->sc_dev); sc->sc_sme->sme_cookie = sc; sc->sc_sme->sme_refresh = ciss_sensor_refresh; + sc->sc_sme->sme_events_timeout = 60; should do the job. Unfortunately I have no hardware to test. > Nevertheless, I think that the big problem with ciss is now > fixed (i.e. it will not hang forever anymore)... It may still wait longer than 30 seconds with the sme_mutex held leading to deadlock. We should use a suitable xs timeout vs. events timeout to make it safe, either increase the event timeout or decrease the xs timeout. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)