bug: notmuch show --decrypt leads to SIGSEGV

classic Classic list List threaded Threaded
7 messages Options
Matt Armstrong Matt Armstrong
Reply | Threaded
Open this post in threaded view
|

bug: notmuch show --decrypt leads to SIGSEGV

I've been able to diagnose a SIGSEGV, and I have a workaround that
satisfies me.  I'm unsure how to fix it, so I'll describe the problem
and leave it at that.

Repro:

% notmuch --version
notmuch 0.25+22~g0967e46 (a recent git @HEAD)
% notmuch show --format=sexp --decrypt thread:000000000002ad2c
-> SIGSEGV

Workaround:

Don't pass --decrypt.  In Emacs, configure notmuch-crypto-process-mime
to shut off crypto processing, or C-u before in notmuch-show before
viewing a problematic thread.

Diagnosis:

mime-node.c's _mime_node_create() can return NULL in various scenarios
yet few to none of its callers appear to handle it properly.  In this
particular case, the NULL is returned here:

#if (GMIME_MAJOR_VERSION < 3)
    if ((GMIME_IS_MULTIPART_ENCRYPTED (part) && node->ctx->crypto->decrypt)
        || (GMIME_IS_MULTIPART_SIGNED (part) && node->ctx->crypto->verify)) {
        GMimeContentType *content_type = g_mime_object_get_content_type (part);
        const char *protocol = g_mime_content_type_get_parameter (content_type, "protocol");
        cryptoctx = notmuch_crypto_get_context (node->ctx->crypto, protocol);
        if (!cryptoctx) {
            return NULL;
        }
    }
#endif

Note above a missing call to talloc_free(node) before the return, which
suggests a kind of bit-rot has set in for the GMIME_MAJOR_VERSION<3
case?  Anyway...

mime_node_child() calls _mime_node_create() and will SIGSEGV:

    node = _mime_node_create (parent, sub);

    if (child == parent->next_child && parent->next_part_num != -1) {
        /* We're traversing in depth-first order.  Record the child's
         * depth-first numbering. */
        node->part_num = parent->next_part_num;
        node->next_part_num = node->part_num + 1;


If I address that by returning NULL from mime_node_child() when
_mime_node_create() does, then the problem cascades to callers.  None of
the callers of mime_node_child() explicitly handle the NULL return case:

mime-node.c: mime_node_t *child = mime_node_child (node, i);
notmuch-show.c:571: format_part_text (ctx, sp, mime_node_child (node, i), indent, params);
notmuch-show.c:622:    format_part_sprinter (ctx, sp, mime_node_child (node, 0), first, TRUE, include_html);
notmuch-show.c:724: format_part_sprinter (ctx, sp, mime_node_child (node, i), i == 0, TRUE, include_html);

..._mime_node_seek_dfs_walk will proceed to SIGSEGV, and so will
format_part_...().
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: bug: notmuch show --decrypt leads to SIGSEGV

Matt Armstrong <[hidden email]> writes:

> I've been able to diagnose a SIGSEGV, and I have a workaround that
> satisfies me.  I'm unsure how to fix it, so I'll describe the problem
> and leave it at that.
>
> Repro:
>
> % notmuch --version
> notmuch 0.25+22~g0967e46 (a recent git @HEAD)
> % notmuch show --format=sexp --decrypt thread:000000000002ad2c
> -> SIGSEGV
Do you have a way for people other than you to reproduce it? I assume
not all threads are a problem for you with --decrypt?

d

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch

signature.asc (671 bytes) Download Attachment
Matt Armstrong Matt Armstrong
Reply | Threaded
Open this post in threaded view
|

Re: bug: notmuch show --decrypt leads to SIGSEGV

David Bremner <[hidden email]> writes:

> Matt Armstrong <[hidden email]> writes:
>
>> I've been able to diagnose a SIGSEGV, and I have a workaround that
>> satisfies me.  I'm unsure how to fix it, so I'll describe the problem
>> and leave it at that.
>>
>> Repro:
>>
>> % notmuch --version
>> notmuch 0.25+22~g0967e46 (a recent git @HEAD)
>> % notmuch show --format=sexp --decrypt thread:000000000002ad2c
>> -> SIGSEGV
>
> Do you have a way for people other than you to reproduce it? I assume
> not all threads are a problem for you with --decrypt?

The times I have encountered this involve email I can't reveal, so a
repro would take some work.  I'm happy to try to dig out specific
details, but I'm at a slight disadvantage here due to unfamiliarity.

I didn't this notice this before, but I am getting this on stderr:

    "Failed to construct pkcs7 context."

So g_mime_gpg_context_new() is returning NULL.

I am running an old Ubuntu variant.  My libgmime is libgmime-2.6-dev.
Looks like nothing in the API contract for g_mime_gpg_context_new()
states that it never returns NULL.  Still, it appears that libgmime is
built with ENABLE_CRYPTOGRAPHY (when I download gmime source and
debbuild it, config.h sets that macro), so I'm at a loss for why that
function would return an error.
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: bug: notmuch show --decrypt leads to SIGSEGV

Matt Armstrong <[hidden email]> writes:

> David Bremner <[hidden email]> writes:
>
>> Matt Armstrong <[hidden email]> writes:
>>
>>> I've been able to diagnose a SIGSEGV, and I have a workaround that
>>> satisfies me.  I'm unsure how to fix it, so I'll describe the problem
>>> and leave it at that.
>>>
>>> Repro:
>>>
>>> % notmuch --version
>>> notmuch 0.25+22~g0967e46 (a recent git @HEAD)
>>> % notmuch show --format=sexp --decrypt thread:000000000002ad2c
>>> -> SIGSEGV
>>
>> Do you have a way for people other than you to reproduce it? I assume
>> not all threads are a problem for you with --decrypt?
>
> The times I have encountered this involve email I can't reveal, so a
> repro would take some work.  I'm happy to try to dig out specific
> details, but I'm at a slight disadvantage here due to unfamiliarity.
>

Understood. If you manage to bisect the commit that introduces the
problem (I suspect the rearrangement to support gmime-3.0, but you never
know), that might be helpful.

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
Matt Armstrong Matt Armstrong
Reply | Threaded
Open this post in threaded view
|

Re: bug: notmuch show --decrypt leads to SIGSEGV

David Bremner <[hidden email]> writes:

> Matt Armstrong <[hidden email]> writes:
>
>> David Bremner <[hidden email]> writes:
>>
>>> Matt Armstrong <[hidden email]> writes:
>>>
>>>> I've been able to diagnose a SIGSEGV, and I have a workaround that
>>>> satisfies me.  I'm unsure how to fix it, so I'll describe the problem
>>>> and leave it at that.
>>>>
>>>> Repro:
>>>>
>>>> % notmuch --version
>>>> notmuch 0.25+22~g0967e46 (a recent git @HEAD)
>>>> % notmuch show --format=sexp --decrypt thread:000000000002ad2c
>>>> -> SIGSEGV
>>>
>>> Do you have a way for people other than you to reproduce it? I assume
>>> not all threads are a problem for you with --decrypt?
>>
>> The times I have encountered this involve email I can't reveal, so a
>> repro would take some work.  I'm happy to try to dig out specific
>> details, but I'm at a slight disadvantage here due to unfamiliarity.
>>
>
> Understood. If you manage to bisect the commit that introduces the
> problem (I suspect the rearrangement to support gmime-3.0, but you never
> know), that might be helpful.

David, your suspicions may have been correct.  The bisect came up with
the following commit.

1fdc08d0ffab9b211861de5d148d0a79eae840bc is the first bad commit
commit 1fdc08d0ffab9b211861de5d148d0a79eae840bc
Author: David Bremner <[hidden email]>
Date:   Sun Jul 16 01:01:43 2017 +0200

    cli/crypto: treat failure to create a crypto context as fatal.
 
    Silently ignoring signed/encrypted parts seems like the wrong idea,
    and it also complicates future gmime-3.0 compatibility changes.
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: bug: notmuch show --decrypt leads to SIGSEGV

Matt Armstrong <[hidden email]> writes:

>> Understood. If you manage to bisect the commit that introduces the
>> problem (I suspect the rearrangement to support gmime-3.0, but you never
>> know), that might be helpful.
>
> David, your suspicions may have been correct.  The bisect came up with
> the following commit.
>
> 1fdc08d0ffab9b211861de5d148d0a79eae840bc is the first bad commit
> commit 1fdc08d0ffab9b211861de5d148d0a79eae840bc
> Author: David Bremner <[hidden email]>
> Date:   Sun Jul 16 01:01:43 2017 +0200
>
>     cli/crypto: treat failure to create a crypto context as fatal.
>  
>     Silently ignoring signed/encrypted parts seems like the wrong idea,
>     and it also complicates future gmime-3.0 compatibility changes.

Do the messages in question actually verify or decrypt with the code
before this commit, or does notmuch just silently ignore a gmime
failure?  Not that I'm claiming SIGSEGV is an appropriate error
reporting mechanism ;).

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: bug: notmuch show --decrypt leads to SIGSEGV

In reply to this post by Matt Armstrong
Matt Armstrong <[hidden email]> writes:

> I've been able to diagnose a SIGSEGV, and I have a workaround that
> satisfies me.  I'm unsure how to fix it, so I'll describe the problem
> and leave it at that.
>

This is fixed by either using gmime 3.0, notmuch 0.25.2, or notmuch master.

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch