[PATCH] relay: fix potential memory leak

[PATCH] relay: fix potential memory leak

[Zhouyi Zhou]

when relay_open_buf fails in relay_open, program will goto free_bufs,
but chan is nowhere freed.

In addition, give warning to users who forget to provide create file
hook. 	 

Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
---
 kernel/relay.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/relay.c b/kernel/relay.c
index 074994b..e0990c7 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -589,6 +589,13 @@ struct rchan *relay_open(const char *base_filename,
 	chan->parent = parent;
 	chan->private_data = private_data;
 	if (base_filename) {
+		if (!cb || !cb->create_buf_file) {
+			printk(KERN_ERR
+			       "relay_open: has base filename without "
+			       "providing hook to create file\n");
+			kfree(chan);
+			return NULL;
+		}
 		chan->has_base_filename = 1;
 		strlcpy(chan->base_filename, base_filename, NAME_MAX);
 	}
@@ -614,6 +621,7 @@ free_bufs:
 
 	kref_put(&chan->kref, relay_destroy_channel);
 	mutex_unlock(&relay_channels_mutex);
+	kfree(chan);
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(relay_open);
-- 
1.9.1

Re: [PATCH] relay: fix potential memory leak

[Andrew Morton]

On Wed,  1 Jun 2016 18:45:27 +0800 Zhouyi Zhou <yizhouzhou@ict.ac.cn> wrote:

> when relay_open_buf fails in relay_open, program will goto free_bufs,
> but chan is nowhere freed.

OK.

> In addition, give warning to users who forget to provide create file
> hook. 	 

Why?  What's the value in this?

If the user didn't provide ->create_buf_file then setup_callbacks()
will provide them with create_buf_file_default_callback() - what's
wrong with that?

Re: Re: [PATCH] relay: fix potential memory leak

[Zhouyi Zhou]

Thanks Andrew for reviewing
> > In addition, give warning to users who forget to provide create file
> > hook. 	 
> 
> Why?  What's the value in this?
> 
> If the user didn't provide ->create_buf_file then setup_callbacks()
> will provide them with create_buf_file_default_callback() - what's
> wrong with that?
> 
The beginners like me will probably call relay_open with base_filename 
and NULL callback or callback without create_buf_file hook. This call 
will fail in sub function relay_open_buf because 
create_buf_file_default_callback returns empty dentry. I guess it will
be good to warn beginners to provide filesystem related create hooks at
earlier stage or they fail without knowing what has happened.

Best wishes
Zhouyi

Re: [PATCH] relay: fix potential memory leak

[Andrew Morton]

On Thu, 2 Jun 2016 09:24:04 +0800 (GMT+08:00) "Zhouyi Zhou" <yizhouzhou@ict.ac.cn> wrote:

> Thanks Andrew for reviewing
> > > In addition, give warning to users who forget to provide create file
> > > hook. 	 
> > 
> > Why?  What's the value in this?
> > 
> > If the user didn't provide ->create_buf_file then setup_callbacks()
> > will provide them with create_buf_file_default_callback() - what's
> > wrong with that?
> > 
> The beginners like me will probably call relay_open with base_filename 
> and NULL callback or callback without create_buf_file hook. This call 
> will fail in sub function relay_open_buf because 
> create_buf_file_default_callback returns empty dentry. I guess it will
> be good to warn beginners to provide filesystem related create hooks at
> earlier stage or they fail without knowing what has happened.

There is no end to the code which we could add to help beginners!  So
let's just fix the bug please, and we can discuss such development aids
separately.