Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1411093 > unrolled thread

[PATCH] relay: fix potential memory leak

Back to article view | Back to linux.kernel

Contents

  [PATCH] relay: fix potential memory leak Zhouyi Zhou <yizhouzhou@ict.ac.cn> - 2016-06-01 13:00 +0200
    Re: [PATCH] relay: fix potential memory leak Andrew Morton <akpm@linux-foundation.org> - 2016-06-02 00:00 +0200
      Re: Re: [PATCH] relay: fix potential memory leak "Zhouyi Zhou" <yizhouzhou@ict.ac.cn> - 2016-06-02 03:30 +0200
        Re: [PATCH] relay: fix potential memory leak Andrew Morton <akpm@linux-foundation.org> - 2016-06-02 04:20 +0200

#1411093 — [PATCH] relay: fix potential memory leak

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

[toc] | [next] | [standalone]

#1411604

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?

[toc] | [prev] | [next] | [standalone]

#1411737

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

[toc] | [prev] | [next] | [standalone]

#1411762

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web