| Message ID | 20170611123714.31292-1-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/06/2017 14:37, Max Reitz wrote: > qemu proper has done so for 13 years > (8a7ddc38a60648257dc0645ab4a05b33d6040063), qemu-img and qemu-io have > done so for four years (526eda14a68d5b3596be715505289b541288ef2a). > Ignoring this signal is especially important in qemu-nbd because > otherwise a client can easily take down the qemu-nbd server by dropping > the connection when the server wants to send something, for example: > > $ qemu-nbd -x foo -f raw -t null-co:// & > [1] 12726 > $ qemu-io -c quit nbd://localhost/bar > can't open device nbd://localhost/bar: No export with name 'bar' available > [1] + 12726 broken pipe qemu-nbd -x foo -f raw -t null-co:// > > In this case, the client sends an NBD_OPT_ABORT and closes the > connection (because it is not required to wait for a reply), but the > server replies with an NBD_REP_ACK (because it is required to reply). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > I tried to find some other reproducer instead of using a qemu client > (e.g. nping -c 1 --tcp-connect localhost -p 10809, which gives the same > pattern of <FIN-ACK, >PSH-ACK, <RST as qemu-io) but to no avail. This > only results in the write being successful and the next read failing; > interestingly, sendmsg()'s man page states that EPIPE is only returned > when the local end is closed. However, I have not found the NBD server > to close that local end anywhere. > > In any case, ignoring SIGPIPE is the right thing to do. We get an EPIPE > anyway, and this is fully sufficient to let us know that the connection > is dead. > --- > qemu-nbd.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index b7ab86b..b2eeedb 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -580,6 +580,10 @@ int main(int argc, char **argv) > sa_sigterm.sa_handler = termsig_handler; > sigaction(SIGTERM, &sa_sigterm, NULL); > > +#ifdef CONFIG_POSIX > + signal(SIGPIPE, SIG_IGN); > +#endif > + > module_call_init(MODULE_INIT_TRACE); > qcrypto_init(&error_fatal); > > Queued, thanks.
On Sun, Jun 11, 2017 at 02:37:14PM +0200, Max Reitz wrote: > qemu proper has done so for 13 years > (8a7ddc38a60648257dc0645ab4a05b33d6040063), qemu-img and qemu-io have > done so for four years (526eda14a68d5b3596be715505289b541288ef2a). > Ignoring this signal is especially important in qemu-nbd because > otherwise a client can easily take down the qemu-nbd server by dropping > the connection when the server wants to send something, for example: > > $ qemu-nbd -x foo -f raw -t null-co:// & > [1] 12726 > $ qemu-io -c quit nbd://localhost/bar > can't open device nbd://localhost/bar: No export with name 'bar' available > [1] + 12726 broken pipe qemu-nbd -x foo -f raw -t null-co:// > > In this case, the client sends an NBD_OPT_ABORT and closes the > connection (because it is not required to wait for a reply), but the > server replies with an NBD_REP_ACK (because it is required to reply). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > I tried to find some other reproducer instead of using a qemu client > (e.g. nping -c 1 --tcp-connect localhost -p 10809, which gives the same > pattern of <FIN-ACK, >PSH-ACK, <RST as qemu-io) but to no avail. This > only results in the write being successful and the next read failing; > interestingly, sendmsg()'s man page states that EPIPE is only returned > when the local end is closed. However, I have not found the NBD server > to close that local end anywhere. > > In any case, ignoring SIGPIPE is the right thing to do. We get an EPIPE > anyway, and this is fully sufficient to let us know that the connection > is dead. > --- > qemu-nbd.c | 4 ++++ > 1 file changed, 4 insertions(+) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 06/11/2017 07:37 AM, Max Reitz wrote: > qemu proper has done so for 13 years > (8a7ddc38a60648257dc0645ab4a05b33d6040063), qemu-img and qemu-io have > done so for four years (526eda14a68d5b3596be715505289b541288ef2a). > Ignoring this signal is especially important in qemu-nbd because > otherwise a client can easily take down the qemu-nbd server by dropping > the connection when the server wants to send something, for example: > > $ qemu-nbd -x foo -f raw -t null-co:// & > [1] 12726 > $ qemu-io -c quit nbd://localhost/bar > can't open device nbd://localhost/bar: No export with name 'bar' available > [1] + 12726 broken pipe qemu-nbd -x foo -f raw -t null-co:// > > In this case, the client sends an NBD_OPT_ABORT and closes the > connection (because it is not required to wait for a reply), but the > server replies with an NBD_REP_ACK (because it is required to reply). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- As mentioned in another thread, I'm trying to figure out if this patch belongs as a third patch to fix CVE-2017-9524, or whether we want to open a second CVE by considering this a slightly different denial-of-service attack than what my patches fixed. At any rate, this is already merged, so it's too late for me to add my R-b, although your analysis was spot-on correct ;)
On 2017-06-27 19:09, Eric Blake wrote: > On 06/11/2017 07:37 AM, Max Reitz wrote: >> qemu proper has done so for 13 years >> (8a7ddc38a60648257dc0645ab4a05b33d6040063), qemu-img and qemu-io have >> done so for four years (526eda14a68d5b3596be715505289b541288ef2a). >> Ignoring this signal is especially important in qemu-nbd because >> otherwise a client can easily take down the qemu-nbd server by dropping >> the connection when the server wants to send something, for example: >> >> $ qemu-nbd -x foo -f raw -t null-co:// & >> [1] 12726 >> $ qemu-io -c quit nbd://localhost/bar >> can't open device nbd://localhost/bar: No export with name 'bar' available >> [1] + 12726 broken pipe qemu-nbd -x foo -f raw -t null-co:// >> >> In this case, the client sends an NBD_OPT_ABORT and closes the >> connection (because it is not required to wait for a reply), but the >> server replies with an NBD_REP_ACK (because it is required to reply). >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- > > As mentioned in another thread, I'm trying to figure out if this patch > belongs as a third patch to fix CVE-2017-9524, or whether we want to > open a second CVE by considering this a slightly different > denial-of-service attack than what my patches fixed. I think nobody would rip our heads off if we added it to it... I think it's similar in the regard that the NBD server tries to send something to a client that is no longer there, so it crashes (aborting in the original case, due to SIGPIPE here). But strictly speaking it's a different issue, even from the user's perspective: In the original case you kill the server using nmap, here you do so using a real NBD client. Hm, not sure, how hard is it to assign a new CVE? O:-) Max
On Wed, Jun 28, 2017 at 04:27:00PM +0200, Max Reitz wrote: > On 2017-06-27 19:09, Eric Blake wrote: > > On 06/11/2017 07:37 AM, Max Reitz wrote: > >> qemu proper has done so for 13 years > >> (8a7ddc38a60648257dc0645ab4a05b33d6040063), qemu-img and qemu-io have > >> done so for four years (526eda14a68d5b3596be715505289b541288ef2a). > >> Ignoring this signal is especially important in qemu-nbd because > >> otherwise a client can easily take down the qemu-nbd server by dropping > >> the connection when the server wants to send something, for example: > >> > >> $ qemu-nbd -x foo -f raw -t null-co:// & > >> [1] 12726 > >> $ qemu-io -c quit nbd://localhost/bar > >> can't open device nbd://localhost/bar: No export with name 'bar' available > >> [1] + 12726 broken pipe qemu-nbd -x foo -f raw -t null-co:// > >> > >> In this case, the client sends an NBD_OPT_ABORT and closes the > >> connection (because it is not required to wait for a reply), but the > >> server replies with an NBD_REP_ACK (because it is required to reply). > >> > >> Signed-off-by: Max Reitz <mreitz@redhat.com> > >> --- > > > > As mentioned in another thread, I'm trying to figure out if this patch > > belongs as a third patch to fix CVE-2017-9524, or whether we want to > > open a second CVE by considering this a slightly different > > denial-of-service attack than what my patches fixed. > > I think nobody would rip our heads off if we added it to it... I think > it's similar in the regard that the NBD server tries to send something > to a client that is no longer there, so it crashes (aborting in the > original case, due to SIGPIPE here). > > But strictly speaking it's a different issue, even from the user's > perspective: In the original case you kill the server using nmap, here > you do so using a real NBD client. Hm, not sure, how hard is it to > assign a new CVE? O:-) Have we issued a patch for CVE-2017-9524 yet ? If so, then we *must* request a new CVE, because vendors will need to track it as an additional fix to backport & ship, if they've already shipped the previous fix. Regards, Daniel
+-- On Tue, 27 Jun 2017, Eric Blake wrote --+ | > $ qemu-nbd -x foo -f raw -t null-co:// & | > [1] 12726 | > $ qemu-io -c quit nbd://localhost/bar | > can't open device nbd://localhost/bar: No export with name 'bar' available | > [1] + 12726 broken pipe qemu-nbd -x foo -f raw -t null-co:// | > | > In this case, the client sends an NBD_OPT_ABORT and closes the | > connection (because it is not required to wait for a reply), but the | > server replies with an NBD_REP_ACK (because it is required to reply). | > | > Signed-off-by: Max Reitz <mreitz@redhat.com> | > --- | | As mentioned in another thread, I'm trying to figure out if this patch | belongs as a third patch to fix CVE-2017-9524, or whether we want to | open a second CVE by considering this a slightly different | denial-of-service attack than what my patches fixed. Yes, this would be a separate issue, as it breaks 'qemu-nbd' server irrespective of whether 'CVE-2017-9524' fix is applied or not. I'll get a CVE assigned and process it further. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
diff --git a/qemu-nbd.c b/qemu-nbd.c index b7ab86b..b2eeedb 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -580,6 +580,10 @@ int main(int argc, char **argv) sa_sigterm.sa_handler = termsig_handler; sigaction(SIGTERM, &sa_sigterm, NULL); +#ifdef CONFIG_POSIX + signal(SIGPIPE, SIG_IGN); +#endif + module_call_init(MODULE_INIT_TRACE); qcrypto_init(&error_fatal);
qemu proper has done so for 13 years (8a7ddc38a60648257dc0645ab4a05b33d6040063), qemu-img and qemu-io have done so for four years (526eda14a68d5b3596be715505289b541288ef2a). Ignoring this signal is especially important in qemu-nbd because otherwise a client can easily take down the qemu-nbd server by dropping the connection when the server wants to send something, for example: $ qemu-nbd -x foo -f raw -t null-co:// & [1] 12726 $ qemu-io -c quit nbd://localhost/bar can't open device nbd://localhost/bar: No export with name 'bar' available [1] + 12726 broken pipe qemu-nbd -x foo -f raw -t null-co:// In this case, the client sends an NBD_OPT_ABORT and closes the connection (because it is not required to wait for a reply), but the server replies with an NBD_REP_ACK (because it is required to reply). Signed-off-by: Max Reitz <mreitz@redhat.com> --- I tried to find some other reproducer instead of using a qemu client (e.g. nping -c 1 --tcp-connect localhost -p 10809, which gives the same pattern of <FIN-ACK, >PSH-ACK, <RST as qemu-io) but to no avail. This only results in the write being successful and the next read failing; interestingly, sendmsg()'s man page states that EPIPE is only returned when the local end is closed. However, I have not found the NBD server to close that local end anywhere. In any case, ignoring SIGPIPE is the right thing to do. We get an EPIPE anyway, and this is fully sufficient to let us know that the connection is dead. --- qemu-nbd.c | 4 ++++ 1 file changed, 4 insertions(+)