GNU bug report logs - #32375
Bug Gzip v1.9

Previous Next

Package: gzip;

Reported by: Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>

Date: Mon, 6 Aug 2018 15:21:02 UTC

Severity: normal

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 32375 in the body.
You can then email your comments to 32375 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Mon, 06 Aug 2018 15:21:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>:
New bug report received and forwarded. Copy sent to bug-gzip <at> gnu.org. (Mon, 06 Aug 2018 15:21:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>
To: <bug-gzip <at> gnu.org>
Subject: Bug Gzip v1.9
Date: Mon, 6 Aug 2018 11:43:30 +0200
[Message part 1 (text/plain, inline)]
Hello.

We believe to have found a bug in gzip's signal handler.
Attached will you find our bug report.
Excited to hear from you!

Greetings from Germany,
Johannes Przybilla

[gzip-report.md (text/markdown, attachment)]

Information forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Mon, 06 Aug 2018 21:48:01 GMT) Full text and rfc822 format available.

Message #8 received at 32375 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>,
 32375 <at> debbugs.gnu.org
Cc: Bdale Garbee <bdale <at> gag.com>
Subject: Re: bug#32375: Bug Gzip v1.9
Date: Mon, 6 Aug 2018 14:47:43 -0700
[Message part 1 (text/plain, inline)]
Thanks for reporting the bug. Although I doubt whether it can occur on any 
practical platform, we should fix it even if it's just theoretical. I installed 
into gzip master on savannah the attached patch, which I hope fixes the problem; 
please give it a try.

The SYMBIOSYS tool did not discover a related theoretical bug with the 
'caught_signals' variable, which also is used inside a signal handler despite 
not being volatile, so you might want to look into that. The attached patch 
should fix this related bug too.

I am CC'ing this to Bdale Garbee, since much of this patch brings in a Gnulib 
replacement for sigaction that I expect nowadays is used only on mingw, and 
Bdale is my mingw guru (see Bug#32305).
[0001-Fix-some-theoretical-races-in-signal-handling.patch (text/x-patch, attachment)]

Information forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Tue, 07 Aug 2018 22:53:01 GMT) Full text and rfc822 format available.

Message #11 received at 32375 <at> debbugs.gnu.org (full text, mbox):

From: Antonio Diaz Diaz <antonio <at> gnu.org>
To: 32375 <at> debbugs.gnu.org
Cc: Bdale Garbee <bdale <at> gag.com>,
 Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>
Subject: Re: bug#32375: Bug Gzip v1.9
Date: Wed, 08 Aug 2018 00:50:41 +0200
Paul Eggert wrote:
> Thanks for reporting the bug. Although I doubt whether it can occur on
> any practical platform, we should fix it even if it's just theoretical.

I have applied to lzip a change similar to yours, but I'm not sure it 
fixes the theoretical problem.


> +static char volatile remove_ofname[MAX_PATH_LEN];

I see two problems with this fix. The first is that it limits filename 
size. It seems that on GNU/Hurd systems there is no limit to the size of 
a file name, so this would place an artificial limit to the use of lzip 
on such systems.

The second is that posix states the following:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03
------------------------------------------------------------------------
If the process is multi-threaded, or if the process is single-threaded 
and a signal handler is executed other than as the result of:
  * The process calling abort(), raise(), kill(), pthread_kill(), or
    sigqueue() to generate a signal that is not blocked
  * A pending signal being unblocked and being delivered before the
    call that unblocked it returns

the behavior is undefined if the signal handler refers to any object 
other than errno with static storage duration other than by assigning a 
value to an object declared as volatile sig_atomic_t, or if the signal 
handler calls any function defined in this standard other than one of 
the functions listed in the following table.
------------------------------------------------------------------------

The last paragraph seems to imply that a signal handler can't read any 
static object, only write to 'volatile sig_atomic_t'. This is how I 
implemented the Ctrl-C handler in ddrescue, but in the case of gzip and 
lzip, polling a sig_atomic_t variable may be inefficient or cause a 
noticeable delay. I find posix too restrictive in this respect.

Any thoughts?


Antonio.




Information forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Wed, 08 Aug 2018 04:57:01 GMT) Full text and rfc822 format available.

Message #14 received at 32375 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Antonio Diaz Diaz <antonio <at> gnu.org>, 32375 <at> debbugs.gnu.org
Cc: Bdale Garbee <bdale <at> gag.com>,
 Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>
Subject: Re: bug#32375: Bug Gzip v1.9
Date: Tue, 7 Aug 2018 21:56:18 -0700
On 08/07/2018 03:50 PM, Antonio Diaz Diaz wrote:
> I find posix too restrictive in this respect. 

Yes, me too.

The file name length limit has been in gzip since forever, and is a
separate issue (e.g. it happens regardless of signals). So if it is to
be fixed it should be a separate bug number.





Information forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Wed, 08 Aug 2018 08:48:02 GMT) Full text and rfc822 format available.

Message #17 received at 32375 <at> debbugs.gnu.org (full text, mbox):

From: Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 32375 <at> debbugs.gnu.org, Antonio Diaz Diaz <antonio <at> gnu.org>,
 Bdale Garbee <bdale <at> gag.com>
Subject: Re: bug#32375: Bug Gzip v1.9
Date: Wed, 8 Aug 2018 09:43:47 +0200
I agree that the POSIX standard is quite restrictive in this respect.
However I believe that is for a good reason.
Performing non-atomic memory operations on static objects in a signal handler can cause problems with reentrancy.
This can lead to undefined behaviour for example in the case of nested signal handler calls that update the same static object.

> Am 08.08.2018 um 06:56 schrieb Paul Eggert <eggert <at> cs.ucla.edu>:
> 
> On 08/07/2018 03:50 PM, Antonio Diaz Diaz wrote:
>> I find posix too restrictive in this respect. 
> 
> Yes, me too.
> 
> The file name length limit has been in gzip since forever, and is a
> separate issue (e.g. it happens regardless of signals). So if it is to
> be fixed it should be a separate bug number.
> 





Information forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Wed, 08 Aug 2018 09:35:02 GMT) Full text and rfc822 format available.

Message #20 received at 32375 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>
Cc: 32375 <at> debbugs.gnu.org, Antonio Diaz Diaz <antonio <at> gnu.org>,
 Bdale Garbee <bdale <at> gag.com>
Subject: Re: bug#32375: Bug Gzip v1.9
Date: Wed, 8 Aug 2018 02:34:07 -0700
Johannes Przybilla wrote:
> This can lead to undefined behaviour for example in the case of nested signal handler calls

These can't happen in gzip, so we should be OK here.




Information forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Wed, 08 Aug 2018 11:14:02 GMT) Full text and rfc822 format available.

Message #23 received at 32375 <at> debbugs.gnu.org (full text, mbox):

From: Antonio Diaz Diaz <antonio <at> gnu.org>
To: Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>
Cc: 32375 <at> debbugs.gnu.org, Bdale Garbee <bdale <at> gag.com>
Subject: Re: bug#32375: Bug Gzip v1.9
Date: Wed, 08 Aug 2018 13:12:04 +0200
Johannes Przybilla wrote:
> I agree that the POSIX standard is quite restrictive in this
> respect. However I believe that is for a good reason. Performing
> non-atomic memory operations on static objects in a signal handler
> can cause problems with reentrancy. This can lead to undefined
> behaviour for example in the case of nested signal handler calls that
> update the same static object.

This is not at all the case in gzip and gzip-like compressors. In them 
the handler is executed just once to perform some cleanup operations 
(mainly remove the partial output file), an then exit. The handler does 
not perform any non-atomic memory operations on static objects, and 
control never returns to the main thread. IMO posix should allow to this 
kind of cleanup handlers read access to any static object.

In fact, as Paul pointed out, any practical platform probably allows 
such read access because many of the functions posix allows to be called 
from a handler require a filename, which in the case of a cleanup 
handler is most probably a static object.




Information forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Sun, 19 Aug 2018 10:35:02 GMT) Full text and rfc822 format available.

Message #26 received at 32375 <at> debbugs.gnu.org (full text, mbox):

From: Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 32375 <at> debbugs.gnu.org, Antonio Diaz Diaz <antonio <at> gnu.org>,
 Bdale Garbee <bdale <at> gag.com>
Subject: Re: bug#32375: Bug Gzip v1.9
Date: Sun, 19 Aug 2018 12:34:22 +0200
Hello Paul.

I only recently got around to check the fix you committed.
It seems to me that after 'volatile_strcpy (remove_ofname, ofname);‘ the 'remove_ofname‘ variable is not used anymore.
The 'xunlink' function in 'remove_output_file' is still called on 'ofname' instead of 'remove_ofname‘.
Therefore the non-volatile access still occurs in the signal handler.
Could you look into that?

Best regards,
Johannes

> Am 08.08.2018 um 11:34 schrieb Paul Eggert <eggert <at> cs.ucla.edu>:
> 
> Johannes Przybilla wrote:
>> This can lead to undefined behaviour for example in the case of nested signal handler calls
> 
> These can't happen in gzip, so we should be OK here.





Information forwarded to bug-gzip <at> gnu.org:
bug#32375; Package gzip. (Sun, 19 Aug 2018 11:19:02 GMT) Full text and rfc822 format available.

Message #29 received at 32375 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de>
Cc: 32375 <at> debbugs.gnu.org, Antonio Diaz Diaz <antonio <at> gnu.org>,
 Bdale Garbee <bdale <at> gag.com>
Subject: Re: bug#32375: Bug Gzip v1.9
Date: Sun, 19 Aug 2018 04:18:50 -0700
[Message part 1 (text/plain, inline)]
Johannes Przybilla wrote:
> Therefore the non-volatile access still occurs in the signal handler.

Good catch, thanks. I installed the attached further patch; please give it a 
try. Signal handlers are such a pain.
[gzip.diff (text/x-patch, attachment)]

bug closed, send any further explanations to 32375 <at> debbugs.gnu.org and Johannes Przybilla <johannes.przybilla <at> rwth-aachen.de> Request was from Paul Eggert <eggert <at> cs.ucla.edu> to control <at> debbugs.gnu.org. (Wed, 30 Mar 2022 00:24:03 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 27 Apr 2022 11:24:12 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 336 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.