On Fri, Feb 28, 2020 at 3:01 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Fri, 2020-02-28 at 12:46 +0000, Colin Ian King wrote: > > Hi, > > > > Static analysis with Coverity has detected a potential issue in the > > following commit in function ceph_redirect_decode(): > > > > commit 205ee1187a671c3b067d7f1e974903b44036f270 > > Author: Ilya Dryomov <ilya.dryomov@xxxxxxxxxxx> > > Date: Mon Jan 27 17:40:20 2014 +0200 > > > > libceph: follow redirect replies from osds > > > > The issue is as follows: > > > > > > 3486 len = ceph_decode_32(p); > > > > Unused value (UNUSED_VALUE) > > assigned_pointer: Assigning value from len to *p here, but that stored > > value is overwritten before it can be used. > > > > 3487 *p += len; /* skip osd_instructions */ > > 3488 > > 3489 /* skip the rest */ > > > > value_overwrite: Overwriting previous write to *p with value from > > struct_end. > > > > 3490 *p = struct_end; > > > > The *p assignment in line 3487 is effectively being overwritten by the > > *p assignment in 3490. Maybe the following is correct: > > > > len = ceph_decode_32(p); > > - p += len; /* skip osd_instructions */ > > + struct_end = *p + len; /* skip osd_instructions */ > > > > /* skip the rest */ > > *p = struct_end; > > > > I'm not familiar with the ceph structure here, so I'm not sure what the > > correct fix would be. > > > > Probably something like this? (untested, of course) > > ---------------------- > > [PATCH] libceph: fix up Coverity warning in ceph_redirect_decode > > We're going to skip to the end of the msg after checking the > object_name anyway, so there is no need to separately decode > the osd instructions that follow it. > > Reported-by: Colin Ian King <colin.king@xxxxxxxxxxxxx> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > --- > net/ceph/osd_client.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index 8ff2856e2d52..51810db4130a 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -3483,9 +3483,6 @@ static int ceph_redirect_decode(void **p, void > *end, > goto e_inval; > } > > - len = ceph_decode_32(p); > - *p += len; /* skip osd_instructions */ > - > /* skip the rest */ > *p = struct_end; > out: Yeah, I have had the same patch in a local branch here since last year: https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg2092861.html I'll make sure to push it out this time ;) Thanks, Ilya
