From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6DCCBA052A; Tue, 10 Nov 2020 07:10:25 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 95C6AAC8D; Tue, 10 Nov 2020 07:10:23 +0100 (CET) Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) by dpdk.org (Postfix) with ESMTP id 3AC5AA932 for ; Tue, 10 Nov 2020 07:10:21 +0100 (CET) Received: by mail-lj1-f196.google.com with SMTP id h23so8862852ljg.13 for ; Mon, 09 Nov 2020 22:10:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=emumba-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=FUfAIKe7Yvlz6JLrUcyNcUsre3W1GYDIpPqk6gpfzhY=; b=wGik64OVACE4bi5TdvFGv9CfalN/bXRKjZVpHx/OwGEXC5My0/3bcwC6baGKrsOQsy 7PIG+u86lDWD0szGre3XKv9c7e0FChFrz5Fhg/PpC2vCxhkgpEG8ThPycSKvWWtP9dpL e3eit9hUQfTvicuR65vkyy1/56X5uxA9acpjxrnVz6fz9SOGqZHp99/+pN+XuHeXo+wv UNXWTbwy9gsOddRULbZEJcEwgT0Hn71LrAmgc+ZXxbxr4lq8XpenjpilAOp9gROlluOp RqLdfA/81xJu+MrwP6hOQAA9SLXMog8/qaB7Em9VWtOzl1t1tdpiKQtLLmkSJJFfQBAv MqFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=FUfAIKe7Yvlz6JLrUcyNcUsre3W1GYDIpPqk6gpfzhY=; b=DE7fnhFwD59sSU68BCHJJOo3mwXwKeJXX+AjtOTnBSR2+zvJMOH0H0Q9ccck/yv+Dp rWIrGAKQBmDKoP+PRiX4dciHCPXBiN/EHyClUCSTsKJj5PNeL8bekHWYlJipvF/7xi1M BnK4cltjybz7+S8RfqaFrUW0/qEXnCIFjJtMRK8B9SLENksA94PCGJpYXYgb5Kprejwd zfe9b7iVJrlhZBmHwYRbNbmJX+HPm30sG4b3h2lwSif8k4zSIxrV6woUxE4AqL4LgT0X LmNxxvLPuJ4JD4Oih8un7DFhkQ0EXl6qUnjBFJ9HL4OjCG21bvkS1iqKUer/kiQGDqu+ w7XA== X-Gm-Message-State: AOAM5331Zj5qY03iEjkAB4ZgSsPseKuHVMprccX0yOtC0VKFeN/xowFr Wcu+N+koEHqKp1lmM5jKlIbaqdK3q1M6TGUbtJmHdw== X-Google-Smtp-Source: ABdhPJyx6C1DfC2LdaoLZdsh7eDMEp7js5N91ooz1PqdeCvkddnWa+wWov+j7twACV+ZNGvv8m6M6f9cFQpoF7ZJmW0= X-Received: by 2002:a2e:6c0e:: with SMTP id h14mr7020744ljc.117.1604988620703; Mon, 09 Nov 2020 22:10:20 -0800 (PST) MIME-Version: 1.0 References: <20201029125339.30916-1-ibtisam.tariq@emumba.com> In-Reply-To: From: Ibtisam Tariq Date: Tue, 10 Nov 2020 11:10:09 +0500 Message-ID: To: David Marchand Cc: "Kovacevic, Marko" , "Ananyev, Konstantin" , "Pattan, Reshma" , "Mcnamara, John" , Cristian Dumitrescu , "Singh, Jasvinder" , "Xia, Chenbo" , Maxime Coquelin , Xiaoyun Li , dev Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH 1/8] examples/fips_validation: enhance getopt_long usage X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hello David, IMHO, it cannot be moved to read_uint16 parser. If we do, we can't verify that the user input value is greater than UINT16 MAX or not on the overflow data. > > + if (data_room_size == 0 || > > + data_room_size > UINT16_MAX) { > > + cryptodev_fips_validate_usage(prgname); > > + return -EINVAL; > > + } The temp variable:data_room_size is necessary to check the overflow of the command line argument. On Thu, Nov 5, 2020 at 1:59 PM David Marchand wrote: > > Hello Ibtisam, > > On Wed, Nov 4, 2020 at 11:00 AM Ibtisam Tariq wrote: > > > + case MBUF_DATAROOM_KEYWORD_NUM: > > > + { > > > + uint32_t data_room_size; > > > > Here, I don't think we need a temp storage. > > If the value is invalid, the parsing and then init will fail. > > You can directly pass &env.mbuf_data_room to parser_read_uint32 and > > check its value. > > > > > > > > > > > > - env.mbuf_data_room = data_room_size; > > > - } else { > > > + if (parser_read_uint32(&data_room_size, > > > + optarg) < 0) { > > > cryptodev_fips_validate_usage(prgname); > > > return -EINVAL; > > > } > > > + > > > + if (data_room_size == 0 || > > > + data_room_size > UINT16_MAX) { > > > + cryptodev_fips_validate_usage(prgname); > > > + return -EINVAL; > > > + } > > > + > > > + env.mbuf_data_room = data_room_size; > > > + > > > break; > > > + } > > > > The type of env.mbuf_data_room is uint16_t and the temp variable type > > is uint32_t. In my opinion, the temp variable size is bigger than > > env.mbuf_data_room to handle overflow value. > > All of it could be moved to a read_uint16 parser. > WDYT? > > > -- > David Marchand > -- - Ibtisam