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
