Messages in this thread | | | From | NeilBrown <> | Date | Fri, 28 Apr 2006 12:04:25 +1000 | Subject | [PATCH INTRO] Re: [RFC] copy_from_user races with readpage | Subject | [PATCH 000 of 2] Introduction |
| |
On Wednesday April 19, akpm@osdl.org wrote: > Chris Mason <mason@suse.com> wrote: > > > > Hello everyone, > > > > I've been working with IBM on a long standing bug where zeros unexpectedly pop > > up during a disk certification test. We tracked it down to copy_from_user. .... > > I'd have thought that a sufficient fix would be to change > __copy_from_user_inatomic() to not do the zeroing, then review all users to > make sure that they cannot leak uninitialised memory.
So I'm following this up and trying to figure out how best to make this "right".
Following are two patches. The first is the result of the suggested "review". The only users of copy_from_user_inatomic that cannot safely lose the zeroing are two separate (but similar:-() implmentations of copy_from_user_iovec These I have 'fixed'.
It is unfortunate that both chose to "know" exactly the difference between the _inatomic and the regular versions, and call _inatomic not in atomic context. It seems to suggest poor interface design, but I'm not sure exactly what the poor choice is.
Also after reading this code I am very aware that on architectures that aren't saddled with highmem (e.g. 64bit) the duplication of copy_from_user is simply wasted icache space. Possibly it might make sense to guard the first _inatomic copy with "if(PageHighMem(page))" which should complie it away to nothing when highmem isn't present.
The second patch changes __copy_from_user_inatomic to not do zeroing in i386. I'm quite open to the possiblity of being told that something I did there is either very silly or very ugly or both. However not being very experienced in arch/asm code I'm not sure what. Constructive criticism very welcome.
If happiness is achieved with these patches, we then need to look at similar patches for powerpc, mips, and sparc.
Thanks for your time.
NeilBrown
[PATCH 001 of 2] Prepare for __copy_from_user_inatomic to not zero missed bytes. [PATCH 002 of 2] Make copy_from_user_inatomic NOT zero the tail on i386 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |