| Message ID | 58FF2D0E0200007800153D5A@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 25, 2017 at 03:03:42AM -0600, Jan Beulich wrote: > Commit 407a3c00ff ("compat/memory: fix build with old gcc") "fixed" a > build issue by switching to the use of uninitialized data. Due to > - the bounding of the uninitialized data item > - the accessed area being outside of Xen space > - arguments being properly verified by the native hypercall function > this is not a security issue. > > Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Hi, On 25/04/17 14:54, Wei Liu wrote: > On Tue, Apr 25, 2017 at 03:03:42AM -0600, Jan Beulich wrote: >> Commit 407a3c00ff ("compat/memory: fix build with old gcc") "fixed" a >> build issue by switching to the use of uninitialized data. Due to >> - the bounding of the uninitialized data item >> - the accessed area being outside of Xen space >> - arguments being properly verified by the native hypercall function >> this is not a security issue. >> >> Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Wei Liu <wei.liu2@citrix.com> Release-acked-by: Julien Grall <julien.grall@arm.com> Cheers,
On 25/04/17 10:03, Jan Beulich wrote: > Commit 407a3c00ff ("compat/memory: fix build with old gcc") "fixed" a > build issue by switching to the use of uninitialized data. Due to > - the bounding of the uninitialized data item > - the accessed area being outside of Xen space > - arguments being properly verified by the native hypercall function > this is not a security issue. > > Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/common/compat/memory.c +++ b/xen/common/compat/memory.c @@ -251,7 +251,7 @@ int compat_memory_op(unsigned int cmd, X unsigned int limit = (COMPAT_ARG_XLAT_SIZE - sizeof(*nat.atpb)) / (sizeof(nat.atpb->idxs.p) + sizeof(nat.atpb->gpfns.p)); /* Use an intermediate variable to suppress warnings on old gcc: */ - unsigned int size = cmp.atpb.size; + unsigned int size; xen_ulong_t *idxs = (void *)(nat.atpb + 1); xen_pfn_t *gpfns = (void *)(idxs + limit); /* @@ -262,8 +262,10 @@ int compat_memory_op(unsigned int cmd, X enum XLAT_add_to_physmap_batch_u u = XLAT_add_to_physmap_batch_u_res0; - if ( copy_from_guest(&cmp.atpb, compat, 1) || - !compat_handle_okay(cmp.atpb.idxs, size) || + if ( copy_from_guest(&cmp.atpb, compat, 1) ) + return -EFAULT; + size = cmp.atpb.size; + if ( !compat_handle_okay(cmp.atpb.idxs, size) || !compat_handle_okay(cmp.atpb.gpfns, size) || !compat_handle_okay(cmp.atpb.errs, size) ) return -EFAULT;
Commit 407a3c00ff ("compat/memory: fix build with old gcc") "fixed" a build issue by switching to the use of uninitialized data. Due to - the bounding of the uninitialized data item - the accessed area being outside of Xen space - arguments being properly verified by the native hypercall function this is not a security issue. Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> x86/32on64: properly honor add-to-physmap-batch's size Commit 407a3c00ff ("compat/memory: fix build with old gcc") "fixed" a build issue by switching to the use of uninitialized data. Due to - the bounding of the uninitialized data item - the accessed area being outside of Xen space - arguments being properly verified by the native hypercall function this is not a security issue. Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/common/compat/memory.c +++ b/xen/common/compat/memory.c @@ -251,7 +251,7 @@ int compat_memory_op(unsigned int cmd, X unsigned int limit = (COMPAT_ARG_XLAT_SIZE - sizeof(*nat.atpb)) / (sizeof(nat.atpb->idxs.p) + sizeof(nat.atpb->gpfns.p)); /* Use an intermediate variable to suppress warnings on old gcc: */ - unsigned int size = cmp.atpb.size; + unsigned int size; xen_ulong_t *idxs = (void *)(nat.atpb + 1); xen_pfn_t *gpfns = (void *)(idxs + limit); /* @@ -262,8 +262,10 @@ int compat_memory_op(unsigned int cmd, X enum XLAT_add_to_physmap_batch_u u = XLAT_add_to_physmap_batch_u_res0; - if ( copy_from_guest(&cmp.atpb, compat, 1) || - !compat_handle_okay(cmp.atpb.idxs, size) || + if ( copy_from_guest(&cmp.atpb, compat, 1) ) + return -EFAULT; + size = cmp.atpb.size; + if ( !compat_handle_okay(cmp.atpb.idxs, size) || !compat_handle_okay(cmp.atpb.gpfns, size) || !compat_handle_okay(cmp.atpb.errs, size) ) return -EFAULT;