OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
Re: ufs_open: don't leak f_buf

From: Martynas Venckus (martynasvenck.us)
Date: Wed Aug 25 2010 - 15:40:25 CDT


On 8/24/10, Miod Vallat <miodonline.fr> wrote:
>> the little operating system we wrote eventually panic'd (overflowed
>> heap); standalone ufs.c implementation we've used leaks f_buf
>> everytime ufs_open fails
>
> This is correct, but you may be leaking fp->f_blk[] as well. What about
> this instead?

yup i think it's better

ok and thx

> Index: ufs.c
> ===================================================================
> RCS file: /cvs/src/sys/lib/libsa/ufs.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 ufs.c
> --- ufs.c 6 Jan 2008 11:17:18 -0000 1.19
> +++ ufs.c 23 Aug 2010 22:03:12 -0000
> -98,6 +98,7 static int read_inode(ino_t, struct open
> static int block_map(struct open_file *, daddr_t, daddr_t *);
> static int buf_read_file(struct open_file *, char **, size_t *);
> static int search_directory(char *, struct open_file *, ino_t *);
> +static int ufs_close_internal(struct file *);
> #ifdef COMPAT_UFS
> static void ffs_oldfscompat(struct fs *);
> #endif
> -526,10 +527,9 ufs_open(char *path, struct open_file *f
> out:
> if (buf)
> free(buf, fs->fs_bsize);
> - if (rc) {
> - free(fp->f_fs, SBSIZE);
> - free(fp, sizeof(struct file));
> - }
> + if (rc)
> + (void)ufs_close_internal(fp);
> +
> return (rc);
> }
>
> -537,11 +537,18 int
> ufs_close(struct open_file *f)
> {
> struct file *fp = (struct file *)f->f_fsdata;
> - int level;
>
> f->f_fsdata = (void *)0;
> if (fp == (struct file *)0)
> return (0);
> +
> + return (ufs_close_internal(fp));
> +}
> +
> +static void
> +ufs_close_internal(struct file *fp)
> +{
> + int level;
>
> for (level = 0; level < NIADDR; level++) {
> if (fp->f_blk[level])