Re: [PATCH 1/5] accel/qaic: tighten bounds checking in encode_message()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 04, 2023 at 03:18:26PM +0530, Pranjal Ramajor Asha Kanojiya wrote:
> 
> 
> On 7/4/2023 2:08 PM, Dan Carpenter wrote:
> > On Tue, Jul 04, 2023 at 11:57:51AM +0530, Pranjal Ramajor Asha Kanojiya wrote:
> > > > diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c
> > > > index 5c57f7b4494e..a51b1594dcfa 100644
> > > > --- a/drivers/accel/qaic/qaic_control.c
> > > > +++ b/drivers/accel/qaic/qaic_control.c
> > > > @@ -748,7 +748,8 @@ static int encode_message(struct qaic_device *qdev, struct manage_msg *user_msg,
> > > >    	int ret;
> > > >    	int i;
> > > > -	if (!user_msg->count) {
> > > > +	if (!user_msg->count ||
> > > > +	    user_msg->len < sizeof(*trans_hdr)) {
> > > >    		ret = -EINVAL;
> > > >    		goto out;
> > > >    	}
> > > > @@ -765,12 +766,13 @@ static int encode_message(struct qaic_device *qdev, struct manage_msg *user_msg,
> > > >    	}
> > > >    	for (i = 0; i < user_msg->count; ++i) {
> > > > -		if (user_len >= user_msg->len) {
> > > > +		if (user_len >= user_msg->len - sizeof(*trans_hdr)) {
> > > If I understand correctly this check is added to verify if we are left with
> > > trans_hdr size of data. In that case '>' comparison operator should be used.
> > 
> > That was there in the original code and I thought about changing it but
> > I don't like changing things which aren't necessary and == is also
> > invalid so I decided to leave it.
> > 
> I see, I understand your concern about not changing unnecessary things but
> '>=' is incorrect for reason mentioned above. We need to change that to '>'

Oh, yes.  You're right.  I will need to resend.

regards,
dan carpenter




[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux