* Bug: SIGABRT if "notmuch dump" output file is not writeable
@ 2019-07-22 18:47 Ralph Seichter
2019-07-22 22:15 ` David Bremner
0 siblings, 1 reply; 6+ messages in thread
From: Ralph Seichter @ 2019-07-22 18:47 UTC (permalink / raw)
To: notmuch
This is what happens when calling "notmuch dump" (version 0.29.1) with
an output file that is not writeable (tested with FISH and BASH):
root > touch /tmp/out
root > ls -l /tmp/out
-rw-r--r-- 1 root root 0 Jul 22 20:36 /tmp/out
nonroot > notmuch dump --output=/tmp/out
Error renaming /tmp/out.kuZ9t5 to /tmp/out: Operation not permitted
double free or corruption (!prev)
fish: “notmuch dump --output=/tmp/out” terminated by signal SIGABRT (Abort)
While it is understood that Notmuch cannot write to the specified output
file, I don't think this should result in something as harsh as SIGABRT.
-Ralph
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: SIGABRT if "notmuch dump" output file is not writeable
2019-07-22 18:47 Bug: SIGABRT if "notmuch dump" output file is not writeable Ralph Seichter
@ 2019-07-22 22:15 ` David Bremner
2019-07-22 23:37 ` Ralph Seichter
0 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2019-07-22 22:15 UTC (permalink / raw)
To: Ralph Seichter, notmuch
Ralph Seichter <abbot@monksofcool.net> writes:
> While it is understood that Notmuch cannot write to the specified output
> file, I don't think this should result in something as harsh as SIGABRT.
I agree it's a bit ugly to look at. At least it prints what the actual
problem is before the abort. The abort seems to be generated from
gzclose_w, so I guess it might just be an error handling bug in libz.
Do you see any database corruption or more serious issues?
d
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: SIGABRT if "notmuch dump" output file is not writeable
2019-07-22 22:15 ` David Bremner
@ 2019-07-22 23:37 ` Ralph Seichter
2019-07-23 14:57 ` David Bremner
0 siblings, 1 reply; 6+ messages in thread
From: Ralph Seichter @ 2019-07-22 23:37 UTC (permalink / raw)
To: David Bremner, notmuch
* David Bremner:
> I agree it's a bit ugly to look at.
Ah, euphemisms. ;-) Personally, I associate "double free or corruption
(!prev)" with memory trouble or situations where a library cannot
recover from an error state and needs to bail out using abort(). Not
being able to (over)write an existing file is not that serious, IMO.
> Do you see any database corruption or more serious issues?
No, and I don't expect any, as I am assuming that "notmuch dump" will
only ever read the DB.
-Ralph
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: SIGABRT if "notmuch dump" output file is not writeable
2019-07-22 23:37 ` Ralph Seichter
@ 2019-07-23 14:57 ` David Bremner
2019-07-23 17:38 ` Ralph Seichter
0 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2019-07-23 14:57 UTC (permalink / raw)
To: Ralph Seichter, notmuch
Ralph Seichter <abbot@monksofcool.net> writes:
> * David Bremner:
>
>> I agree it's a bit ugly to look at.
>
> Ah, euphemisms. ;-) Personally, I associate "double free or corruption
> (!prev)" with memory trouble or situations where a library cannot
> recover from an error state and needs to bail out using abort(). Not
> being able to (over)write an existing file is not that serious, IMO.
Yes, but that's a message / abort from deep within libz. So odds of our
being able to fix it are pretty small. Checking for permissions before
hand would just introduce a race condition (I _think_).
>
>> Do you see any database corruption or more serious issues?
>
> No, and I don't expect any, as I am assuming that "notmuch dump" will
> only ever read the DB.
That's true, but it does take write lock on the database so that
one dumps a consistent state.
d
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: SIGABRT if "notmuch dump" output file is not writeable
2019-07-23 14:57 ` David Bremner
@ 2019-07-23 17:38 ` Ralph Seichter
2019-07-23 18:53 ` David Bremner
0 siblings, 1 reply; 6+ messages in thread
From: Ralph Seichter @ 2019-07-23 17:38 UTC (permalink / raw)
To: David Bremner, notmuch
* David Bremner:
> that's a message / abort from deep within libz. So odds of our being
> able to fix it are pretty small.
Based on a quick glance on notmuch_database_dump() in notmuch-dump.c, it
seems to me that SIGABRT occurs in line 351:
350: if (ret != EXIT_SUCCESS && output)
351: (void) gzclose_w (output);
gzclose_w(output) has already been called in line 332, before Notmuch
attempts to rename the temp file to the output file. At that point,
'output' should be set to null as it is being checked later, but that
erroneously only happens in case the close operation fails.
The rename in line 341 fails because of permission/ownership issues,
'ret' contains the error code for that, 'output' is still non-null, so
gzclose_w is called again -- ergo boom.
I have not tested or debugged this so far, it is just a Mark-I-Eyeball
analysis. I think I got it right, though. If you agree, I can provide a
fix, which I will actually test.
-Ralph
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug: SIGABRT if "notmuch dump" output file is not writeable
2019-07-23 17:38 ` Ralph Seichter
@ 2019-07-23 18:53 ` David Bremner
0 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2019-07-23 18:53 UTC (permalink / raw)
To: Ralph Seichter, notmuch
Ralph Seichter <abbot@monksofcool.net> writes:
> gzclose_w(output) has already been called in line 332, before Notmuch
> attempts to rename the temp file to the output file. At that point,
> 'output' should be set to null as it is being checked later, but that
> erroneously only happens in case the close operation fails.
>
> The rename in line 341 fails because of permission/ownership issues,
> 'ret' contains the error code for that, 'output' is still non-null, so
> gzclose_w is called again -- ergo boom.
That sounds plausible to me. Thanks for thinking about this longer than
I did. I'll be happy to look at a patch.
d
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-27 23:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-22 18:47 Bug: SIGABRT if "notmuch dump" output file is not writeable Ralph Seichter
2019-07-22 22:15 ` David Bremner
2019-07-22 23:37 ` Ralph Seichter
2019-07-23 14:57 ` David Bremner
2019-07-23 17:38 ` Ralph Seichter
2019-07-23 18:53 ` David Bremner
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).