dsingh
dsingh

Reputation: 270

Potential memory leak in linux kernel?

While doing static analysis of linux kernel for memory leaks, I came across an interesting scenario where i am not able to find the de allocation of a variable. The allocation is happening in the following function(using kmalloc call) as below:

static int mounts_open_common(struct inode *inode, struct file *file,
              int (*show)(struct seq_file *, struct vfsmount *)){
  struct proc_mounts *p;

  //some code//
  *p = kmalloc(sizeof(struct proc_mounts), GFP_KERNEL);**
  file->private_data = &p->m;//the allocated variable is escaped to file structure
  //some code

}

I expect this allocated memory to be fixed at:

static int mounts_release(struct inode *inode, struct file *file)
{
    struct proc_mounts *p = proc_mounts(file->private_data);
    path_put(&p->root);
    put_mnt_ns(p->ns);
    return seq_release(inode, file);
}

But it seems this function is accessing the allocated variable to free some its internel members but not the variable 'p' itself. So where is this variable's memory is freed? If it is supposed to free in mounts_release function then its a potential memory leak.

Upvotes: 3

Views: 842

Answers (1)

nos
nos

Reputation: 229184

If you look at seq_release:

int seq_release(struct inode *inode, struct file *file)
{
        struct seq_file *m = file->private_data;
        kvfree(m->buf);
        kfree(m);
        return 0;
}

It effectivly does kfree(file->private_data)

Now, file->private_data is set up in mounts_open_common as

file->private_data = &p->m;

That's the p which is kmalloc'd in your question. The m member isn't a pointer, so that shouldn't be allowed to be freed. However, it's the 1. member of struct proc_mounts

struct proc_mounts {
        struct seq_file m;
        struct mnt_namespace *ns;
        struct path root;
        int (*show)(struct seq_file *, struct vfsmount *);
        void *cached_mount;
        u64 cached_event;
        loff_t cached_index;
};

So seq_release() performs kfree() on the address of the m member, that's the same address as was obtained with p = kmalloc(sizeof(struct proc_mounts), GFP_KERNEL);

I guess that's not very friendly to a static analyzer. But there's no memory leak.

Upvotes: 7

Related Questions