Re: [dpdk-dev] [PATCH v5 1/4] examples/multi_process/client_server_mp: check port validity

[Stephen Hemminger <stephen@networkplumber.org>]

On Sun, 4 Aug 2019 08:31:54 +0000
Matan Azrad <matan@mellanox.com> wrote:

> > > >  	/* convert parameter to a number and verify */
> > > >  	pm = strtoul(portmask, &end, 16);
> > > > -	if (end == NULL || *end != '\0' || pm == 0)
> > > > +	if (end == NULL || *end != '\0' || pm > UINT16_MAX || pm == 0)  
> > >
> > > Why pm > UINT16_MAX ? should be something like > (1 <<  
> > RTE_MAX_ETHPORTS) - 1.  
> > > And need to be sure pm type can hold RTE_MAX_ETHPORTS bits,  
> > otherwise port 0 may unlikely be all the time visible in the loop below.  
> > >  
> > 
> > The DPDK assumes a lot of places that unsigned long will hold a port mask.  
> 
> So, all are bugs, no?

I don't think 32 bit build is that well tested. But yes a mask
needs to hold 64 ports.


> > If some extra bits are set, the error is visible later when the bits are leftover
> > after finding ports.  
> 
> Yes, but if there is a valid port which its port id is bigger than the portmask bits number - port 0 will be all the time visible in the check -> bug.
> 
> > The original code had worse problems, it would not catch invalid pm values at
> > all and truncate silently.  
> 
> Yes, maybe, but I really don't understand why you chose to limit for 16 ports, where this number come from?
> So, my approach here, 2 options:

The problem  here was my mistake for not having wide enough portmask.