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 <david.march...@redhat.com> wrote:
>
> Hello Ibtisam,
>
> On Wed, Nov 4, 2020 at 11:00 AM Ibtisam Tariq <ibtisam.ta...@emumba.com>
> 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
