On Fri, Apr 4, 2014 at 4:44 PM, Andres Perera <andre...@zoho.com> wrote:
> On Fri, Apr 4, 2014 at 2:12 PM, Philip Guenther <guent...@gmail.com> wrote:
>> On Fri, Apr 4, 2014 at 11:27 AM, Marco Pfatschbacher <m...@mailq.de> wrote:
>>> On Thu, Apr 03, 2014 at 05:19:45PM -0600, Theo de Raadt wrote:
>>>> Interesting. Can we take bath approaches?
>>>
>>> I don't see why we should not.
>>>
>>>> Is there a reason to not expose either error?
>>>
>>> I thought it might break some legacy stuff regarding
>>> tapes and such. But since no one spoke up...
>>>
>>> OK?
>>>
>>>> > pax does not exit with an error if the processed
>>>> > archive is truncated:
>>>> >
>>>> > # (cd / && tar zcf - bsd | dd count=128 2>/dev/null | tar tzf -)
>>>> > bsd
>>>> > gzip: stdin: Input/output error
>>>> > tar: End of archive volume 1 reached
>>>> > gzip: stdout: Broken pipe
>>>> > tar: Failed write to archive volume: 1: Broken pipe
>>>> > # echo $?
>>>> > 0
>>>> >
>>>> >
>>>> > There's two ways to fix this.
>>>> > 1) take the exit code of gzip into account:
>>>> >
>>>> > Index: ar_io.c
>>>> > ===================================================================
>>>> > RCS file: /cvs/src/bin/pax/ar_io.c,v
>>>> > retrieving revision 1.44
>>>> > diff -u -p -p -u -r1.44 ar_io.c
>>>> > --- ar_io.c 11 Jan 2014 05:36:26 -0000 1.44
>>>> > +++ ar_io.c 28 Mar 2014 14:09:37 -0000
>>>> > @@ -337,8 +337,11 @@ ar_close(void)
>>>> > (void)close(arfd);
>>>> >
>>>> > /* Do not exit before child to ensure data integrity */
>>>> > - if (zpid > 0)
>>>> > + if (zpid > 0) {
>>>> > waitpid(zpid, &status, 0);
>>>> > + if (WIFEXITED(status) && WEXITSTATUS(status))
>>>> > + exit_val = 1;
>>>> > + }
>>
>> Hmm, should that be
>> if (!WIFEXITED(status) || WEXITSTATUS(status))
>
> This is incorrect because a process can conclude its work before
> terminating because of a signal.
>
> To demonstrate I can use the OP's pipeline:
...
> This is only an example. Like other processes, gzip is subject to the race.
Yes, it's possible that it could have actually completed but died
right after it wrote its last byte, but *you can't tell*. Let's say
you're running
tar -czf backup.tgz /data
and gzip is killed before it can process the last pipe-buffer worth of
data (perhaps it's getting SIGXFSZ or SIGXCPU). I think you're
suggesting that tar should hope for the best (the data *might* be
fine) and return success instead of exiting with a failure to let you
know that no, it isn't sure your backup is complete. I think tar must
not return success when it has reason to believe the backup wasn't
completely written correctly.
That said, there is a bug in my suggestion: tar itself will kill a
child gzip/bzip2/whatever in the list or extract "match first only"
case:
if ((act == LIST || act == EXTRACT) && nflag && zpid > 0)
kill(zpid, SIGINT);
Since tar has all the data it can possible need in that case and is
about to exit anyway, the transient zombie doesn't matter, so just
suppress the waitpid() then:
--- ar_io.c 11 Jan 2014 05:36:26 -0000 1.44
+++ ar_io.c 5 Apr 2014 00:51:06 -0000
@@ -331,14 +331,20 @@ ar_close(void)
* for a quick extract/list, pax frequently exits before the child
* process is done
*/
- if ((act == LIST || act == EXTRACT) && nflag && zpid > 0)
+ if ((act == LIST || act == EXTRACT) && nflag && zpid > 0) {
kill(zpid, SIGINT);
+ zpid = -1;
+ }
(void)close(arfd);
/* Do not exit before child to ensure data integrity */
- if (zpid > 0)
+ if (zpid > 0) {
waitpid(zpid, &status, 0);
+ if (!WIFEXITED(status) || WEXITSTATUS(status))
+ exit_val = 1;
+ }
+
if (vflag && (artyp == ISTAPE)) {
(void)fputs("done.\n", listf);
Philip Guenther
