On 12 Mar 2015, at 20:00, Christos Zoulas <[email protected]> wrote:
> On Mar 12, 12:20pm, [email protected] ("J. Hannken-Illjes") wrote: > -- Subject: Re: DoS attack against TCP services > > | Now we have a deadlock, softlck/0 waits for the mutex and therefore > | callouts will no longer be processed and ciss holds the mutex and waits > | for a callout through cv_timedwait. > > Thanks for looking into it! Part of the ciss_ioctl_vol() (the pdid part) > does things with XS_CTL_POLL so that it does not involve any mutexes. It > would be simple to change the ldid part to do the same. Should we do that? > > | Taking a closer look at the poll loop from sys/dev/ic/ciss.c:537 ... this > | code looks wrong in many aspects: > | > | - Sleeping up to 60 seconds in a function used by a callout is wrong. > > Yes, but many disk drivers seem to violate that. How do we fix this? > Making a separate thread that updates statistics for each driver seems > suboptimal? > > | - Examining variables here we get: tick =3D 10000, etick =3D 16000, > | tohz =3D 6000 and i =3D 5999999. As tick is constant (us per hz) > | this loop might run for 5999999*60 seconds! > > I committed a fix for this. Now it should only sleep up to 60 seconds. Looks like you made it worse. "tick" is constant, for HZ == 100 it is 10000 so you now have etick = tick + tohz -> etick = 10000 + tohz and then tohz = etick - tick -> tohz = (10000 + tohz) - 10000 -> tohz = tohz so ciss_wait() may now loop forever. Are you looking for hardclock_ticks? -- J. Hannken-Illjes - [email protected] - TU Braunschweig (Germany)
