Messages in this thread | | | From | Dongliang Mu <> | Date | Wed, 29 Jun 2022 09:56:44 +0800 | Subject | Re: [PATCH] gfs2: fix overread in the strlcpy of init_names |
| |
On Wed, Jun 29, 2022 at 9:33 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote: > > On Tue, Jun 28, 2022 at 8:06 PM Andreas Gruenbacher <agruenba@redhat.com> wrote: > > > > Dongliang Mu, > > > > On Tue, Jun 28, 2022 at 8:10 AM Dongliang Mu <dzm91@hust.edu.cn> wrote: > > > From: Dongliang Mu <mudongliangabcd@gmail.com> > > > > > > In init_names, strlcpy will overread the src string as the src string is > > > less than GFS2_FSNAME_LEN(256). > > > > > > Fix this by modifying strlcpy back to snprintf, reverting > > > the commit 00377d8e3842. > > > > ... if the source string isn't NULL-terminated. But in that case, the > > code will still do the same thing with this patch. In other words, > > this doesn't fix anything. So let's check for NULL termination > > instead. > > Partially yes. Even if the source string is NULL-terminated, strlcpy > will invoke memcpy to overread the adjacent memory of source string as > the specified length is longer than source string.
The above statement is incorrect after I double check the implementation of strlcpy.
The correct fix should be NULL-termination check of src string.
> > > > > Thanks, > > Andreas > > > > > Fixes: 00377d8e3842 ("[GFS2] Prefer strlcpy() over snprintf()") > > > Reported-by: syzkaller <syzkaller@googlegroups.com> > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > > > --- > > > fs/gfs2/ops_fstype.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c > > > index c9b423c874a3..ee29b50d39b9 100644 > > > --- a/fs/gfs2/ops_fstype.c > > > +++ b/fs/gfs2/ops_fstype.c > > > @@ -383,8 +383,8 @@ static int init_names(struct gfs2_sbd *sdp, int silent) > > > if (!table[0]) > > > table = sdp->sd_vfs->s_id; > > > > > > - strlcpy(sdp->sd_proto_name, proto, GFS2_FSNAME_LEN); > > > - strlcpy(sdp->sd_table_name, table, GFS2_FSNAME_LEN); > > > + snprintf(sdp->sd_proto_name, GFS2_FSNAME_LEN, "%s", proto); > > > + snprintf(sdp->sd_table_name, GFS2_FSNAME_LEN, "%s", table); > > > > > > table = sdp->sd_table_name; > > > while ((table = strchr(table, '/'))) > > > -- > > > 2.35.1 > > > > >
| |