qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiri Daněk (JIRA) <j...@apache.org>
Subject [jira] [Commented] (PROTON-1573) Undefined behavior in some calls to memmove in Proton
Date Thu, 05 Oct 2017 12:41:00 GMT

    [ https://issues.apache.org/jira/browse/PROTON-1573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16192810#comment-16192810
] 

Jiri Daněk commented on PROTON-1573:
------------------------------------

This one here https://github.com/apache/qpid-proton/commit/feafb6c80b121f0a7ce87ea09c9b2f0f2d0fadf5

> Undefined behavior in some calls to memmove in Proton
> -----------------------------------------------------
>
>                 Key: PROTON-1573
>                 URL: https://issues.apache.org/jira/browse/PROTON-1573
>             Project: Qpid Proton
>          Issue Type: Bug
>          Components: proton-c
>    Affects Versions: proton-c-0.18.0
>            Reporter: Jiri Daněk
>            Assignee: Alan Conway
>            Priority: Trivial
>             Fix For: proton-c-0.18.0
>
>
> Proton sometimes calls to memmove with arguments memmove(?, null, 0), that is, the source
pointer is null and length is zero.
> According to UndefinedBehaviorSanitizer tool (UBSan), this has undefined behavior.
> {noformat}
> SUMMARY: AddressSanitizer: undefined-behavior /home/jdanek/Work/repos/qpid-proton/proton-c/src/core/codec.c:268:35
in 
> /home/jdanek/Work/repos/qpid-proton/proton-c/src/core/framing.c:97:39: runtime error:
null pointer passed as argument 2, which is declared to never be null
> {noformat}
> It can be "fixed" in the following manner, but I think it is probably not worth worrying
about, unless the code can be somehow restructured that {{n = 0}} is caught sooner. I verified
the fix by running UBSan again. Nothing was reported this time.
> {noformat}
> diff --git a/proton-c/src/core/buffer.c b/proton-c/src/core/buffer.c
> index c3015f49..f47acd6d 100644
> --- a/proton-c/src/core/buffer.c
> +++ b/proton-c/src/core/buffer.c
> @@ -170,8 +170,12 @@ int pn_buffer_append(pn_buffer_t *buf, const char *bytes, size_t
size)
>    size_t tail_space = pni_buffer_tail_space(buf);
>    size_t n = pn_min(tail_space, size);
>  
> +  if (n > 0) {
>    memmove(buf->bytes + tail, bytes, n);
> +  }
> +  if (size - n > 0) { 
>    memmove(buf->bytes, bytes + n, size - n);
> +  }
>  
>    buf->size += size;
>  
> diff --git a/proton-c/src/core/framing.c b/proton-c/src/core/framing.c
> index 09bf4bba..d2c355f8 100644
> --- a/proton-c/src/core/framing.c
> +++ b/proton-c/src/core/framing.c
> @@ -94,7 +94,9 @@ size_t pn_write_frame(char *bytes, size_t available, pn_frame_t frame)
>      bytes[5] = frame.type;
>      pn_i_write16(&bytes[6], frame.channel);
>  
> -    memmove(bytes + AMQP_HEADER_SIZE, frame.extended, frame.ex_size);
> +    if (frame.ex_size > 0) {
> +        memmove(bytes + AMQP_HEADER_SIZE, frame.extended, frame.ex_size);
> +    }
>      memmove(bytes + 4*doff, frame.payload, frame.size);
>      return size;
>    } else {
> {noformat}
> After brief Googling, I believe that although this really is an undefined behavior, it
is reasonable to expect that any real-world implementation of memmove will be a simple noop
as long as {{n = 0}}, which it is always the case. (https://stackoverflow.com/a/5243068/1047788)
> Probably best to ignore this.
> The tests that uncover this are for example
> proton_tests.engine.ConnectionTest.test_capabilities:
> ../proton-c/src/core/framing.c:97:5: runtime error: null pointer passed as argument 2,
which is declared to never be null
> proton_tests.engine.CreditTest.testPartialDrain:
> ../proton-c/src/core/buffer.c:173:3: runtime error: null pointer passed as argument 2,
which is declared to never be null
> ../proton-c/src/core/buffer.c:174:3: runtime error: null pointer passed as argument 2,
which is declared to never be null



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Mime
View raw message