All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools/xenstored: Check number of strings passed to do_control()
@ 2017-10-27 16:32 Pawel Wieczorkiewicz
  2017-10-27 16:37 ` [PATCH for-4.10] " Ian Jackson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Pawel Wieczorkiewicz @ 2017-10-27 16:32 UTC (permalink / raw)
  To: xen-devel
  Cc: jgross, wei.liu2, julien.grall, ian.jackson, mpohlack,
	Pawel Wieczorkiewicz, doebel

It is possible to send a zero-string message body to xenstore's
XS_CONTROL handling function. Then the number of strings is used
for an array allocation. This leads to a crash in strcmp() in a
CONTROL sub-command invocation loop.
The output of xs_count_string() should be verified and all 0 or
negative values should be rejected with an EINVAL. At least the
sub-command name must be specified.

The xenstore crash can only be triggered from within dom0 (there
is a check in do_control() rejecting all non-dom0 requests with
an EACCES).

Testing: reproduced with the following command:
python -c 'print 16*"\x00"' | nc -U $XENSTORED_RUNDIR/socket

Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Reviewed-by: Martin Pohlack <mpohlack@amazon.de>
---
 tools/xenstore/xenstored_control.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index 7c14911..e4b8aa9 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -184,6 +184,8 @@ int do_control(struct connection *conn, struct buffered_data *in)
 		return EACCES;
 
 	num = xs_count_strings(in->buffer, in->used);
+	if (num < 1)
+		return EINVAL;
 	vec = talloc_array(in, char *, num);
 	if (!vec)
 		return ENOMEM;
-- 
1.8.3.1

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH for-4.10] tools/xenstored: Check number of strings passed to do_control()
  2017-10-27 16:32 [PATCH] tools/xenstored: Check number of strings passed to do_control() Pawel Wieczorkiewicz
@ 2017-10-27 16:37 ` Ian Jackson
  2017-11-13 16:03   ` Julien Grall
  2017-10-27 17:21 ` [PATCH] " Juergen Gross
  2017-10-30 16:32 ` Wei Liu
  2 siblings, 1 reply; 5+ messages in thread
From: Ian Jackson @ 2017-10-27 16:37 UTC (permalink / raw)
  To: Pawel Wieczorkiewicz
  Cc: jgross, wei.liu2, julien.grall, xen-devel, mpohlack, doebel

Pawel Wieczorkiewicz writes ("[PATCH] tools/xenstored: Check number of strings passed to do_control()"):
> It is possible to send a zero-string message body to xenstore's
> XS_CONTROL handling function. Then the number of strings is used
> for an array allocation. This leads to a crash in strcmp() in a
> CONTROL sub-command invocation loop.
> The output of xs_count_string() should be verified and all 0 or
> negative values should be rejected with an EINVAL. At least the
> sub-command name must be specified.
> 
> The xenstore crash can only be triggered from within dom0 (there
> is a check in do_control() rejecting all non-dom0 requests with
> an EACCES).

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

(Added the for-4.10 tag to the Subject.)

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tools/xenstored: Check number of strings passed to do_control()
  2017-10-27 16:32 [PATCH] tools/xenstored: Check number of strings passed to do_control() Pawel Wieczorkiewicz
  2017-10-27 16:37 ` [PATCH for-4.10] " Ian Jackson
@ 2017-10-27 17:21 ` Juergen Gross
  2017-10-30 16:32 ` Wei Liu
  2 siblings, 0 replies; 5+ messages in thread
From: Juergen Gross @ 2017-10-27 17:21 UTC (permalink / raw)
  To: Pawel Wieczorkiewicz, xen-devel
  Cc: mpohlack, wei.liu2, julien.grall, ian.jackson, doebel

On 27/10/17 18:32, Pawel Wieczorkiewicz wrote:
> It is possible to send a zero-string message body to xenstore's
> XS_CONTROL handling function. Then the number of strings is used
> for an array allocation. This leads to a crash in strcmp() in a
> CONTROL sub-command invocation loop.
> The output of xs_count_string() should be verified and all 0 or
> negative values should be rejected with an EINVAL. At least the
> sub-command name must be specified.
> 
> The xenstore crash can only be triggered from within dom0 (there
> is a check in do_control() rejecting all non-dom0 requests with
> an EACCES).
> 
> Testing: reproduced with the following command:
> python -c 'print 16*"\x00"' | nc -U $XENSTORED_RUNDIR/socket
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Martin Pohlack <mpohlack@amazon.de>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] tools/xenstored: Check number of strings passed to do_control()
  2017-10-27 16:32 [PATCH] tools/xenstored: Check number of strings passed to do_control() Pawel Wieczorkiewicz
  2017-10-27 16:37 ` [PATCH for-4.10] " Ian Jackson
  2017-10-27 17:21 ` [PATCH] " Juergen Gross
@ 2017-10-30 16:32 ` Wei Liu
  2 siblings, 0 replies; 5+ messages in thread
From: Wei Liu @ 2017-10-30 16:32 UTC (permalink / raw)
  To: Pawel Wieczorkiewicz
  Cc: jgross, wei.liu2, julien.grall, ian.jackson, xen-devel, mpohlack, doebel

On Fri, Oct 27, 2017 at 04:32:15PM +0000, Pawel Wieczorkiewicz wrote:
> It is possible to send a zero-string message body to xenstore's
> XS_CONTROL handling function. Then the number of strings is used
> for an array allocation. This leads to a crash in strcmp() in a
> CONTROL sub-command invocation loop.
> The output of xs_count_string() should be verified and all 0 or
> negative values should be rejected with an EINVAL. At least the
> sub-command name must be specified.
> 
> The xenstore crash can only be triggered from within dom0 (there
> is a check in do_control() rejecting all non-dom0 requests with
> an EACCES).
> 
> Testing: reproduced with the following command:
> python -c 'print 16*"\x00"' | nc -U $XENSTORED_RUNDIR/socket
> 
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
> Reviewed-by: Martin Pohlack <mpohlack@amazon.de>

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH for-4.10] tools/xenstored: Check number of strings passed to do_control()
  2017-10-27 16:37 ` [PATCH for-4.10] " Ian Jackson
@ 2017-11-13 16:03   ` Julien Grall
  0 siblings, 0 replies; 5+ messages in thread
From: Julien Grall @ 2017-11-13 16:03 UTC (permalink / raw)
  To: Ian Jackson, Pawel Wieczorkiewicz
  Cc: mpohlack, jgross, wei.liu2, doebel, xen-devel

Hi,

Apologies for the late answer, I missed the e-mail in my inbox.

On 10/27/2017 05:37 PM, Ian Jackson wrote:
> Pawel Wieczorkiewicz writes ("[PATCH] tools/xenstored: Check number of strings passed to do_control()"):
>> It is possible to send a zero-string message body to xenstore's
>> XS_CONTROL handling function. Then the number of strings is used
>> for an array allocation. This leads to a crash in strcmp() in a
>> CONTROL sub-command invocation loop.
>> The output of xs_count_string() should be verified and all 0 or
>> negative values should be rejected with an EINVAL. At least the
>> sub-command name must be specified.
>>
>> The xenstore crash can only be triggered from within dom0 (there
>> is a check in do_control() rejecting all non-dom0 requests with
>> an EACCES).
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> (Added the for-4.10 tag to the Subject.)

Release-acked-by: Julien Grall <julien.grall@linaro.org>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-11-13 16:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-27 16:32 [PATCH] tools/xenstored: Check number of strings passed to do_control() Pawel Wieczorkiewicz
2017-10-27 16:37 ` [PATCH for-4.10] " Ian Jackson
2017-11-13 16:03   ` Julien Grall
2017-10-27 17:21 ` [PATCH] " Juergen Gross
2017-10-30 16:32 ` Wei Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.