_odo_packet_copy_to_packet() will fail if either the source or target
offset/length is out of range. In this case it can only happen if out_pkt
is shorter than pkt. The problem here is that this code should be using
odp_packet_alloc() instead of odp_buffer_alloc() (strong typechecking would
flag this as an error) and specifying the correct length of the packet it
needs. Here's the fuller context of the code in question:
/* Resolve output buffer */
if (ODP_PACKET_INVALID == params->out_pkt)
if (ODP_BUFFER_POOL_INVALID != session->output_pool)
params->out_pkt =
odp_buffer_alloc(session->output_pool);
if (params->pkt != params->out_pkt) {
if (odp_unlikely(ODP_PACKET_INVALID == params->out_pkt))
abort();
_odp_packet_copy_to_packet(params->pkt, 0, params->out_pkt, 0,
odp_packet_len(params->pkt));
if (completion_event == odp_packet_to_buffer(params->pkt))
completion_event =
odp_packet_to_buffer(params->out_pkt);
odp_packet_free(params->pkt);
params->pkt = ODP_PACKET_INVALID;
}
The correct code should be:
/* Resolve output buffer */
if (ODP_PACKET_INVALID == params->out_pkt)
if (ODP_BUFFER_POOL_INVALID != session->output_pool)
params->out_pkt =
odp_packet_alloc(session->output_pool,
odp_packet_len(params->pkt));
if (params->pkt != params->out_pkt) {
if (odp_unlikely(ODP_PACKET_INVALID == params->out_pkt))
abort();
_odp_packet_copy_to_packet(params->pkt, 0, params->out_pkt, 0,
odp_packet_len(params->pkt));
if (completion_event == odp_packet_to_buffer(params->pkt))
completion_event =
odp_packet_to_buffer(params->out_pkt);
odp_packet_free(params->pkt);
params->pkt = ODP_PACKET_INVALID;
}
With this change _odp_packet_copy_to_packet() cannot return a non-zero
value.
So this is a bug in the original patch and should be corrected as noted
above. Maxim: did you want to change your patch or should I post this bug
fix?
On Tue, Dec 23, 2014 at 7:10 AM, Ola Liljedahl <[email protected]>
wrote:
> On 23 December 2014 at 12:39, Maxim Uvarov <[email protected]>
> wrote:
> > CID 85004: Unchecked return value (CHECKED_RETURN)
> > Calling "_odp_packet_copy_to_packet" without checking
> > return value (as is done elsewhere 5 out of 6 times).
> >
> > Signed-off-by: Maxim Uvarov <[email protected]>
> > ---
> > platform/linux-generic/odp_crypto.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/platform/linux-generic/odp_crypto.c
> b/platform/linux-generic/odp_crypto.c
> > index 13c5556..09adda1 100644
> > --- a/platform/linux-generic/odp_crypto.c
> > +++ b/platform/linux-generic/odp_crypto.c
> > @@ -350,6 +350,7 @@ odp_crypto_operation(odp_crypto_op_params_t *params,
> > enum crypto_alg_err rc_auth = ODP_CRYPTO_ALG_ERR_NONE;
> > odp_crypto_generic_session_t *session;
> > odp_crypto_generic_op_result_t *result;
> > + int ret;
> >
> > *posted = 0;
> > session = (odp_crypto_generic_session_t
> *)(intptr_t)params->session;
> > @@ -362,8 +363,11 @@ odp_crypto_operation(odp_crypto_op_params_t *params,
> > if (params->pkt != params->out_pkt) {
> > if (odp_unlikely(ODP_PACKET_INVALID == params->out_pkt))
> > abort();
> > - _odp_packet_copy_to_packet(params->pkt, 0,
> params->out_pkt, 0,
> > - odp_packet_len(params->pkt));
> > + ret = _odp_packet_copy_to_packet(params->pkt, 0,
> params->out_pkt, 0,
> > +
> odp_packet_len(params->pkt));
> When can odp_packet_copy_to_packet() fail? Because we need to allocate
> more buffer/segments? Buffer exhaustion is a non-fatal error in a
> networking application and we cannot abort the program. Roll back this
> operation and return an error to the caller.
>
> > + if (0 != ret)
> > + abort();
> > +
> > if (completion_event ==
> odp_packet_to_buffer(params->pkt))
> > completion_event =
> > odp_packet_to_buffer(params->out_pkt);
> > --
> > 1.8.5.1.163.gd7aced9
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > [email protected]
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
