[PATCH] Fixed leaks in _netfs_translator_callback2_fn

[James Clarke]

* libnetfs/trans-callback.c (_netfs_translator_callback2_fn): Fixed leaking
iouser and peropen structs on error.
---
 libnetfs/trans-callback.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libnetfs/trans-callback.c b/libnetfs/trans-callback.c
index 3f1aee6..7816bd1 100644
--- a/libnetfs/trans-callback.c
+++ b/libnetfs/trans-callback.c
@@ -67,7 +67,10 @@ _netfs_translator_callback2_fn (void *cookie1, void 
*cookie2, int flags,
 
   po = netfs_make_peropen (node, flags, cookie2);
   if (! po)
-return errno;
+{
+  iohelp_free_iouser (user);
+  return errno;
+}
 
   cred = netfs_make_protid (po, user);
   if (cred)
@@ -79,6 +82,7 @@ _netfs_translator_callback2_fn (void *cookie1, void *cookie2, 
int flags,
 }
   else
 {
+  netfs_release_peropen (po);
   iohelp_free_iouser (user);
   return errno;
 }
-- 
2.7.1

Re: [PATCH] Fixed leaks in _netfs_translator_callback2_fn

[James Clarke]

> On 7 Feb 2016, at 23:10, Samuel Thibault  wrote:
> Flávio Cruz, on Sun 07 Feb 2016 23:57:25 +0100, wrote:
>> Maybe here we should do it as follows:
>> 
>> err = errno;
>> netfs_release_peropen (po);
>> iohelp_free_iouser (user);
>> return err;
> 
> Yes, you never know what they could be doing to errno.
> 
> Samuel

Does that include changing

>   po = netfs_make_peropen (node, flags, cookie2);
>   if (! po)
> -return errno;
> +{
> +  iohelp_free_iouser (user);
> +  return errno;
> +}

to

>   po = netfs_make_peropen (node, flags, cookie2);
>   if (! po)
> -return errno;
> +{
> +  err = errno;
> +  iohelp_free_iouser (user);
> +  return err;
> +}

or can we assume the free functions don’t set errno?

James


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH] Fixed leaks in _netfs_translator_callback2_fn

[Samuel Thibault]

James Clarke, on Sun 07 Feb 2016 23:13:06 +, wrote:
> > On 7 Feb 2016, at 23:10, Samuel Thibault  wrote:
> > Flávio Cruz, on Sun 07 Feb 2016 23:57:25 +0100, wrote:
> >> Maybe here we should do it as follows:
> >> 
> >> err = errno;
> >> netfs_release_peropen (po);
> >> iohelp_free_iouser (user);
> >> return err;
> > 
> > Yes, you never know what they could be doing to errno.
> > 
> > Samuel
> 
> Does that include changing
> 
> >   po = netfs_make_peropen (node, flags, cookie2);
> >   if (! po)
> > -return errno;
> > +{
> > +  iohelp_free_iouser (user);
> > +  return errno;
> > +}
> 
> to
> 
> >   po = netfs_make_peropen (node, flags, cookie2);
> >   if (! po)
> > -return errno;
> > +{
> > +  err = errno;
> > +  iohelp_free_iouser (user);
> > +  return err;
> > +}

I'd say so too, yes.

Samuel

Re: [PATCH] Fixed leaks in _netfs_translator_callback2_fn

[Samuel Thibault]

Flávio Cruz, on Sun 07 Feb 2016 23:57:25 +0100, wrote:
> Maybe here we should do it as follows:
> 
> err = errno;
> netfs_release_peropen (po);
> iohelp_free_iouser (user);
> return err;

Yes, you never know what they could be doing to errno.

Samuel

Re: [PATCH] Fixed leaks in _netfs_translator_callback2_fn

[Flávio Cruz]

Thanks for noticing!


> @@ -79,6 +82,7 @@ _netfs_translator_callback2_fn (void *cookie1, void
> *cookie2, int flags,
>  }
>else
>  {
> +  netfs_release_peropen (po);
>iohelp_free_iouser (user);
>return errno;
>  }


Maybe here we should do it as follows:

err = errno;
netfs_release_peropen (po);
iohelp_free_iouser (user);
return err;

I've seen this pattern a few times in other files since the release calls
may reassign errno.


-- 
Flávio Cruz / flavioc...@gmail.com