linux-kernel - Re: [PATCH] IDE: Fix HDIO_DRIVE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <873an3tixa.fsf@denkblock.local>
Date:	Tue, 24 Jun 2008 09:02:09 +0200
From:	Elias Oltmanns <eo@...ensachen.de>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org,
	Randy Dunlap <randy.dunlap@...cle.com>
Subject: Re: [PATCH] IDE: Fix HDIO_DRIVE_RESET handling

Alan Cox <alan@...rguk.ukuu.org.uk> wrote:
>> in various ways.  Most importantly, it is treated as an out of band
>> request in an illegal way which may very likely lead to system lock ups.
>> Use the drive's request queue to avoid this problem (and fix a locking
>> issue for free along the way).
>
> It was always designed to be, and used out of band. One of the important
> uses of the ioctl is to abort a running command when an interface has
> jammed up. If you end up queueing it behind that command you've lost most
> of the reason for the ioctl anyway (and you might as well just remove it
> really given SG_IO exists).

Well, I can see your point. In fact, there really doesn't seem to be an
alternative to the out of band approach for the purposes you described.
Now, I even think that I could perhaps fix the request aborting properly
and restore the original behaviour. Moreover, I may very likely live to
regret having removed ide_abort() and friends when implementing disk
shock protection in the IDE layer. Maybe I should try to send an
alternative patch for discussion. On the other hand I don't see the
equivalent for HDIO_DRIVE_RESET in libata which makes me wonder whether
this ioctl has actually been used in real life for the purposes you
described.

>
> Other than the command aborting bit, it looks a good idea - that code
>has
> always been racy and raced against timer handlers, irq handlers and if
> neither of them got it then a speed changedown raced the lot 8(

My idea to solve this would be roughly this: Change ide_set_handler to
leave the ->handler and ->expiry members alone if they have been set on
entry. If a request is being processed by the time a HDIO_DRIVE_RESET
ioctl is received, these callbacks will be changed so the reset sequence
will be started on the next interrupt, timeout, or when the ->busy flag
is cleared. I'm not quite sure yet whether things will work out the way
I want them to and I don't know whether HDIO_DRIVE_RESET actually
justifies the effort since I don't knowof an equivalent in libata
anyway. But as I said, it might come in handy for other purposes.

Comments?

Elias
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/