Re: [PATCH v4l-utils] libdvbv5: leaks and double free in dvb_fe_open_fname()

[Mauro Carvalho Chehab <mchehab+samsung@kernel.org>]

Gregor,

This patch messed with all branches since stable-1.12. I applied the revert
patch already on all affected stable branches.

We should probably release a new fix for them soon.

Sorry for not looking this earlier.. I got some vacations earlier 
this month.

Regards,
Mauro

Em Fri, 26 Apr 2019 12:13:44 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> escreveu:

> Em Sun, 17 Mar 2019 16:32:20 +0000
> Sean Young <sean@mess.org> escreveu:
> 
> > dvb_fe_open_fname() takes ownership of fname if the function succeeds, but
> > also in two of the error paths (e.g. if the ioctl FE_GET_PROPERTY fails).
> > 
> > Adjust dvb_fe_open_fname() so it copies fname rather than taking ownership
> > (and passing that to params). This makes the code cleaner.  
> 
> Just reverted this patch from stable-1.16, as it breaks Kaffeine.
> 
> There are two reports about the issue:
> 
> 	https://bugs.kde.org/show_bug.cgi?id=406145
>         https://bugzilla.redhat.com/show_bug.cgi?id=1695023
> 
> I was able to reproduce it locally.
> 
> So, better to keep a possible memory leak than to cause apps
> to not function anymore.
> 
> > 
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> >  lib/libdvbv5/dvb-dev-local.c |  2 +-
> >  lib/libdvbv5/dvb-fe.c        | 18 ++++++++----------
> >  2 files changed, 9 insertions(+), 11 deletions(-)
> > 
> > diff --git a/lib/libdvbv5/dvb-dev-local.c b/lib/libdvbv5/dvb-dev-local.c
> > index e98b967a..2de9a614 100644
> > --- a/lib/libdvbv5/dvb-dev-local.c
> > +++ b/lib/libdvbv5/dvb-dev-local.c
> > @@ -467,7 +467,7 @@ static struct dvb_open_descriptor
> >  			flags &= ~O_NONBLOCK;
> >  		}
> >  
> > -		ret = dvb_fe_open_fname(parms, strdup(dev->path), flags);
> > +		ret = dvb_fe_open_fname(parms, dev->path, flags);
> >  		if (ret) {
> >  			free(open_dev);
> >  			return NULL;
> > diff --git a/lib/libdvbv5/dvb-fe.c b/lib/libdvbv5/dvb-fe.c
> > index 5dcf492e..7f634766 100644
> > --- a/lib/libdvbv5/dvb-fe.c
> > +++ b/lib/libdvbv5/dvb-fe.c
> > @@ -133,7 +133,6 @@ struct dvb_v5_fe_parms *dvb_fe_open_flags(int adapter, int frontend,
> >  					  int flags)
> >  {
> >  	int ret;
> > -	char *fname;
> >  	struct dvb_device *dvb;
> >  	struct dvb_dev_list *dvb_dev;
> >  	struct dvb_v5_fe_parms_priv *parms = NULL;
> > @@ -153,7 +152,6 @@ struct dvb_v5_fe_parms *dvb_fe_open_flags(int adapter, int frontend,
> >  		dvb_dev_free(dvb);
> >  		return NULL;
> >  	}
> > -	fname = strdup(dvb_dev->path);
> >  
> >  	if (!strcmp(dvb_dev->bus_addr, "platform:dvbloopback")) {
> >  		logfunc(LOG_WARNING, _("Detected dvbloopback"));
> > @@ -161,14 +159,10 @@ struct dvb_v5_fe_parms *dvb_fe_open_flags(int adapter, int frontend,
> >  	}
> >  
> >  	dvb_dev_free(dvb);
> > -	if (!fname) {
> > -		logfunc(LOG_ERR, _("fname calloc: %s"), strerror(errno));
> > -		return NULL;
> > -	}
> > +
> >  	parms = calloc(sizeof(*parms), 1);
> >  	if (!parms) {
> >  		logfunc(LOG_ERR, _("parms calloc: %s"), strerror(errno));
> > -		free(fname);
> >  		return NULL;
> >  	}
> >  	parms->p.verbose = verbose;
> > @@ -183,7 +177,7 @@ struct dvb_v5_fe_parms *dvb_fe_open_flags(int adapter, int frontend,
> >  	if (use_legacy_call)
> >  		parms->p.legacy_fe = 1;
> >  
> > -	ret = dvb_fe_open_fname(parms, fname, flags);
> > +	ret = dvb_fe_open_fname(parms, dvb_dev->path, flags);
> >  	if (ret < 0) {
> >  		dvb_v5_free(parms);
> >  		return NULL;
> > @@ -203,7 +197,6 @@ int dvb_fe_open_fname(struct dvb_v5_fe_parms_priv *parms, char *fname,
> >  	fd = open(fname, flags, 0);
> >  	if (fd == -1) {
> >  		dvb_logerr(_("%s while opening %s"), strerror(errno), fname);
> > -		free(fname);
> >  		return -errno;
> >  	}
> >  
> > @@ -224,7 +217,12 @@ int dvb_fe_open_fname(struct dvb_v5_fe_parms_priv *parms, char *fname,
> >  		}
> >  	}
> >  
> > -	parms->fname = fname;
> > +	parms->fname = strdup(fname);
> > +	if (!parms->fname) {
> > +		dvb_logerr(_("fname calloc: %s"), strerror(errno));
> > +		return -errno;
> > +	}
> > +
> >  	parms->fd = fd;
> >  	parms->fe_flags = flags;
> >  	parms->dvb_prop[0].cmd = DTV_API_VERSION;  
> 
> 
> 
> Thanks,
> Mauro



Thanks,
Mauro