Re: read after free in notmuch new

[David Bremner]

David Bremner  writes:

> Tomi Ollila  writes:
>
>> To me it looks like replacing g_hash_table_insert() with 
>> g_hash_table_replace() would do the trick.
>>
>> (or even g_hash_table_add()!)
>>
>> One has to read the documentation a bit (and compare the docstrings of
>> these 2 functions to guess the missing pieces) to get some understanding to
>> this...
>>
>
> Hi Tomi;
>
> Thanks for the suggestion. Unfortunately in my experiments it just
> shifts the invalid memory access to a different piece of memory. I think
> the problem is that a pointer to the previous copy of that key also
> leaked a reference via last_ref, so when we kill that via
> g_hash_table_replace it causes the same problem.
>

Thinking a bit more about it, at least in this case the pointer that was
just inserted remains valid after insertion, and can be
talloc_strdup'ed, i.e.

diff --git a/lib/database.cc b/lib/database.cc
index f0bfe566..eddb780c 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -652,7 +652,7 @@ parse_references (void *ctx,
ref = _parse_message_id (ctx, refs, );
 
if (ref && strcmp (ref, message_id)) {
-   g_hash_table_insert (hash, ref, NULL);
+   g_hash_table_add (hash, ref);
last_ref = ref;
}
 }
@@ -661,7 +661,7 @@ parse_references (void *ctx,
  * reference to the database.  We should avoid making a message
  * its own parent, thus the above check.
  */
-return last_ref;
+return talloc_strdup(ctx, last_ref);
 }
 
 notmuch_status_t

I'll let valgrind chew on that for a bit and see what it says.


___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: bug report

[David Bremner]

Andy Wills  writes:

>
> andy@andy-x200:~/work/talks/2017/icps$ notmuch new 
> Processed 230 total files in 7s (29 files/sec.).
> Added 212 new messages to the database. Removed 85 messages. Detected 18 file 
> renames.
> Error: A Xapian exception occurred flushing database: Value in posting list 
> too large.
>
> ...and it seems that, since I've been getting this error, new emails
> are not reliably being found by, for example M-x notmuch in emacs.
>

Hi Andy;

What version of notmuch is that? From the error message it sounds
pre-2014. That particular code was rewritten in notmuch 0.19, so if my
guess is correct it might be fixed by newer notmuch.

d

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: read after free in notmuch new

[David Bremner]

Tomi Ollila  writes:

> To me it looks like replacing g_hash_table_insert() with 
> g_hash_table_replace() would do the trick.
>
> (or even g_hash_table_add()!)
>
> One has to read the documentation a bit (and compare the docstrings of
> these 2 functions to guess the missing pieces) to get some understanding to
> this...
>

Hi Tomi;

Thanks for the suggestion. Unfortunately in my experiments it just
shifts the invalid memory access to a different piece of memory. I think
the problem is that a pointer to the previous copy of that key also
leaked a reference via last_ref, so when we kill that via
g_hash_table_replace it causes the same problem.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

bug report

[Andy Wills]

Hi,

I've been consistently getting an error of this form:

andy@andy-x200:~/work/talks/2017/icps$ notmuch new 
Processed 230 total files in 7s (29 files/sec.).
Added 212 new messages to the database. Removed 85 messages. Detected 18 file 
renames.
Error: A Xapian exception occurred flushing database: Value in posting list too 
large.

...and it seems that, since I've been getting this error, new emails
are not reliably being found by, for example M-x notmuch in emacs.

Can anyone help?

Best

Andy Wills
Psychology
Plymouth University, UK

-
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: read after free in notmuch new

[Tomi Ollila]

On Tue, Feb 21 2017, David Bremner  wrote:

> David Bremner  writes:
>
>> David Bremner  writes:
>>
>>> I haven't had a chance to really track this down, but it seems there is
>>> a memory error in notmuch new (or a maybe false positive from valgrind).
>>>
>>> Attached is the log from running "make memory-test OPTIONS=--medium" on
>>> current git master (0e037c34).
>>>
>>> It looks like we talloc the message_id string with the message object as
>>> parent, but it somehow outlives the message object.
>>
>> Sorry, that had a few commits beyond master.
>>
>> master (08343d3d) gives essentially the same log.
>>
>
> The log says the relevent piece of memory was freed at line 655 of 
> database.cc, which
> is the g_hash_table_insert in the code 
>
>   ref = _parse_message_id (ctx, refs, );
>
>   if (ref && strcmp (ref, message_id)) {
>   g_hash_table_insert (hash, ref, NULL);
>   last_ref = ref;
>   }
>
>
> According to the docs for g_hash_table_insert
>
>If the key already exists in the GHashTable its current value is
>replaced with the new value. If you supplied a value_destroy_func
>when creating the GHashTable, the old value is freed using that
>function. If you supplied a key_destroy_func when creating the
>GHashTable, the passed key is freed using that function.
>
> Since we do pass a key_destroy_func, it seems we are being naughty by
> returning last_ref just below.

To me it looks like replacing g_hash_table_insert() with 
g_hash_table_replace() would do the trick.

(or even g_hash_table_add()!)

One has to read the documentation a bit (and compare the docstrings of
these 2 functions to guess the missing pieces) to get some understanding to
this...

Tomi

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: notmuch version/Python bindings

[Sebastian Spaeth]

Hi there,
just tried to delete the notmuch packagr on pypi. While I am listed as the 
author, I do not own the package and it lists "Package Index Owner: Julian".

It might be that I have already transferred the ownership of the package, but I 
don't remember who Julian is. Has anyone else a clue?

Sebastian Spaeth
-- 
Sent from my mobile phone. Please excuse brevity.

Am 18. Februar 2017 16:14:49 MEZ schrieb David Bremner :
>Sebastian Spaeth  writes:
>
>> Hi there, I did stop using notmuch, true. Let me know if I should
>hand
>> over administration or if I shouldnsimply delete the package on pypi.
>>
>
>Hi Sebastian;
>
>Thanks for the followup.  If no-one steps up to maintain by the end of
>next week, I'd say just delete them from pypi.
>
>I don't really think that "the notmuch project" needs to be involved in
>deciding who, if anyone, maintains a pypi package (anymore than we
>micromanage who maintains notmuch in various linux distros). I have a
>vague memory that Justus (nominally in charge of the python bindings)
>was not interested in pypi, but I could be wrong. In any case Justus
>not
>very active lately either.
>
>d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: Proposed fix for test failures due to long socket paths

[David Bremner]

David Bremner  writes:

> Amadeusz Żołnowski found a bug in the test suite that causes gpg
> failures when the path of the test directory is sufficiently long.
> His current solution in Gentoo is to move the sockets into /tmp. It
> seems cleaner to enable gnupg's built in /run based short pathed
> sockets.  As far as I know the gpgconf option used here is available
> in gnupg 2.1.13 and later. Amadeusz reported that the problem was
> "fixed" in Gentoo by downgrading gnupg to 2.1.15. So while I'm not
> 100% sure, it seems like a fix for very recent gnupg is all that is
> required. Testing of these patches with older gpg would be
> particularly welcome.

Pushed the series (with Tomi's changes) to release and master

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: some issues with emacs 25

[David Bremner]

Matthew Lear  writes:


> Thanks David. Yes it does. After recompiling the v25 lisp with these
> changes, I'm unable to reproduce the problems with both the test emails I
> sent you. Wonderful :-)
> Are you going to raise this with upstream?
> Cheers,
>  Matt

Yes, I've filed

 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25828

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: some issues with emacs 25

[Matthew Lear]

On 18 Feb 2017 01:01, "David Bremner"  wrote:


So I _finally_ got around to looking at these, and I think it's roughly
the same shr bug as before but some different functions.

I could actually only duplicate the bug with emacs-reply-fail-ec
message, but that was fixed by the following patch against the emacs-25
branch. Does this patch fix both failures for you?

diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index 6c35a33c9c..2bc37c64bd 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -1993,6 +1993,9 @@ shr-pixel-buffer-width
 (if (get-buffer-window)
(car (window-text-pixel-size nil (point-min) (point-max)))
   (save-window-excursion
+;; Avoid errors if the selected window is a dedicated one,
+;; and they just want to insert a document into it.
+(set-window-dedicated-p nil nil)
(set-window-buffer nil (current-buffer))
(car (window-text-pixel-size nil (point-min) (point-max)))

@@ -2036,6 +2039,9 @@ shr-render-td-1
(shr-indentation 0))
(shr-descend dom))
   (save-window-excursion
+;; Avoid errors if the selected window is a dedicated one,
+;; and they just want to insert a document into it.
+(set-window-dedicated-p nil nil)
(set-window-buffer nil (current-buffer))
(unless fill
  (setq natural-width


Thanks David. Yes it does. After recompiling the v25 lisp with these
changes, I'm unable to reproduce the problems with both the test emails I
sent you. Wonderful :-)
Are you going to raise this with upstream?
Cheers,
 Matt
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch