Chris Wright
2005-Jun-27 18:41 UTC
[Xen-devel] [PATCH] proper bounds check in do_set_gdt entry point
Unless I missed something, not bounds checking entries in do_set_gdt is a security hole. Signed-off-by: Chris Wright <chrisw@osdl.org> --- 1.184/xen/arch/x86/mm.c 2005-06-23 02:35:56 -07:00 +++ edited/xen/arch/x86/mm.c 2005-06-27 11:37:30 -07:00 @@ -2442,6 +2442,9 @@ long do_set_gdt(unsigned long *frame_lis unsigned long frames[16]; long ret; + if ( entries > FIRST_RESERVED_GDT_ENTRY ) + return -EINVAL; + if ( copy_from_user(frames, frame_list, nr_pages * sizeof(unsigned long)) ) return -EFAULT; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2005-Jun-27 19:21 UTC
Re: [Xen-devel] [PATCH] proper bounds check in do_set_gdt entry point
On 27 Jun 2005, at 19:41, Chris Wright wrote:> Unless I missed something, not bounds checking entries in do_set_gdt is > a security hole. > > Signed-off-by: Chris Wright <chrisw@osdl.org>The check happens in set_gdt(). do_set_gdt is just a wrapper with a copy_from_user plus conditional tlb flush. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Chris Wright
2005-Jun-27 20:00 UTC
Re: [Xen-devel] [PATCH] proper bounds check in do_set_gdt entry point
* Keir Fraser (Keir.Fraser@cl.cam.ac.uk) wrote:> > On 27 Jun 2005, at 19:41, Chris Wright wrote: > > >Unless I missed something, not bounds checking entries in do_set_gdt is > >a security hole. > > > >Signed-off-by: Chris Wright <chrisw@osdl.org> > > The check happens in set_gdt(). do_set_gdt is just a wrapper with a > copy_from_user plus conditional tlb flush.I know, but copy_from_user has no sane limits. This is stack smash. -chris _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2005-Jun-27 20:09 UTC
Re: [Xen-devel] [PATCH] proper bounds check in do_set_gdt entry point
On 27 Jun 2005, at 21:00, Chris Wright wrote:>> The check happens in set_gdt(). do_set_gdt is just a wrapper with a >> copy_from_user plus conditional tlb flush. > > I know, but copy_from_user has no sane limits. This is stack smash. > -chrisGood point. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel