On 20.04.2017 23:25, Maxim Uvarov wrote: > On 04/20/17 19:12, Bill Fischofer wrote: >> On Mon, Mar 13, 2017 at 6:44 AM, Dmitry Eremin-Solenikov < >> [email protected]> wrote: >> >>> Add proper handling for errors returned by odp_packet_copy_from_pkt(). >>> >>> Signed-off-by: Dmitry Eremin-Solenikov <[email protected]> >>> >> >> Reviewed-by: Bill Fischofer <[email protected]> >> >> >>> --- >>> platform/linux-generic/odp_crypto.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/platform/linux-generic/odp_crypto.c >>> b/platform/linux-generic/odp_crypto.c >>> index 54b222fd..675b3e25 100644 >>> --- a/platform/linux-generic/odp_crypto.c >>> +++ b/platform/linux-generic/odp_crypto.c >>> @@ -886,11 +886,17 @@ odp_crypto_operation(odp_crypto_op_param_t *param, >>> } >>> >>> if (param->pkt != param->out_pkt) { >>> - (void)odp_packet_copy_from_pkt(param->out_pkt, >>> + int ret; >>> + >>> + ret = odp_packet_copy_from_pkt(param->out_pkt, >>> 0, >>> param->pkt, >>> 0, >>> odp_packet_len(param->pkt)); >>> + if (odp_unlikely(ret < 0)) { >>> + ODP_DBG("Copy failed.\n"); > > > if you are here than packet param->out_pkt was allocated, it needs to be > freed and set in invalid to prevent packet leaks. > > it looks like also other non zero returns in that function needs the > same change.
There is connected interesting question, which should be though wrt. all 'packet-consuming' functions. Should such functions always consume and free incoming packet? IOW: - odp_crypto_operation() returned -1. Should the app free inbound packet afterwards? - odp_ipsec_*() returned -1. Should inbound packets be freed by an app? Would it be sane for the app to assume that it should re-read inbound packet/packet array from params(!) and free all non-INVALID packets (or requeque them, etc). -- With best wishes Dmitry
