On 12/23/2014 04:28 PM, Bill Fischofer wrote:
_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?
Bill I think Coverity complained that _odp_packet_copy_to_packet() is
int function and return value is not checked. Even it it's returns only
0 Coverety might complain about it. Fill free to send your version of patch.
Maxim.
On Tue, Dec 23, 2014 at 7:10 AM, Ola Liljedahl
<[email protected] <mailto:[email protected]>> wrote:
On 23 December 2014 at 12:39, Maxim Uvarov
<[email protected] <mailto:[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]
<mailto:[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] <mailto:[email protected]>
> http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected] <mailto:[email protected]>
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
