sergiotarxz/nuttx
sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

pipes: use priv refs instead of inode to resolve memleak

Browse Source 
thread1                 thread2
open pipe               open pipe
close()
-> pipecommon_close()
  -> check inode refs
  -> do NOT free dev    close()
                        -> pipecommon_close()
                          -> check inode refs
                          -> do NOT free dev
-> inode_release
   inode refs--
                        -> inode_release
                           inode refs--

Then, you will see the pipe hasn't free its resource, memleak

Resolve:
replace the inode refs with priv refs

Signed-off-by: ligd <liguiding1@xiaomi.com>
This commit is contained in:
ligd 2023-11-22 17:59:30 +08:00 committed by Alan Carvalho de Assis
parent f811b78d8c
commit 1a21445877
3 changed files with 26 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
drivers/pipes/pipe.c
View File

@ -48,8 +48,6 @@
* Private Function Prototypes
****************************************************************************/
static int pipe_close(FAR struct file *filep);
static int pipe_mmap(FAR struct file *filep,
FAR struct mm_map_entry_s *entry);
@ -60,7 +58,7 @@ static int pipe_mmap(FAR struct file *filep,
static const struct file_operations g_pipe_fops =
{
pipecommon_open, /* open */
pipe_close, /* close */
pipecommon_close, /* close */
pipecommon_read, /* read */
pipecommon_write, /* write */
NULL, /* seek */
@ -101,31 +99,6 @@ static inline int pipe_allocate(void)
return ret;
}
/****************************************************************************
* Name: pipe_close
****************************************************************************/
static int pipe_close(FAR struct file *filep)
{
FAR struct inode *inode = filep->f_inode;
FAR struct pipe_dev_s *dev = inode->i_private;
int ret;
DEBUGASSERT(dev);
/* Perform common close operations */
ret = pipecommon_close(filep);
if (ret == 0 && inode->i_crefs == 1)
{
/* Release the pipe when there are no further open references to it. */
pipecommon_freedev(dev);
}
return ret;
}
/****************************************************************************
* Name: pipe_mmap
****************************************************************************/
@ -170,6 +143,8 @@ static int pipe_register(size_t bufsize, int flags,
return -ENOMEM;
}
PIPE_UNLINK(dev->d_flags);
/* Register the pipe device */
ret = register_pipedriver(devname, &g_pipe_fops, 0666, (FAR void *)dev);

40
drivers/pipes/pipe_common.c
View File

@ -94,12 +94,11 @@ FAR struct pipe_dev_s *pipecommon_allocdev(size_t bufsize)
/* Allocate a private structure to manage the pipe */
dev = kmm_malloc(sizeof(struct pipe_dev_s));
dev = kmm_zalloc(sizeof(struct pipe_dev_s));
if (dev)
{
/* Initialize the private structure */
memset(dev, 0, sizeof(struct pipe_dev_s));
nxmutex_init(&dev->d_bflock);
nxsem_init(&dev->d_rdsem, 0, 0);
nxsem_init(&dev->d_wrsem, 0, 0);
@ -150,7 +149,7 @@ int pipecommon_open(FAR struct file *filep)
* is first opened.
*/
if (inode->i_crefs == 1 && !circbuf_is_init(&dev->d_buffer))
if (dev->d_crefs == 0)
{
ret = circbuf_init(&dev->d_buffer, NULL, dev->d_bufsize);
if (ret < 0)
@ -160,6 +159,8 @@ int pipecommon_open(FAR struct file *filep)
}
}
dev->d_crefs++;
/* If opened for writing, increment the count of writers on the pipe
* instance.
*/
@ -304,7 +305,7 @@ int pipecommon_close(FAR struct file *filep)
FAR struct pipe_dev_s *dev = inode->i_private;
int ret;
DEBUGASSERT(dev && filep->f_inode->i_crefs > 0);
DEBUGASSERT(dev && dev->d_crefs > 0);
/* Make sure that we have exclusive access to the device structure.
* NOTE: close() is supposed to return EINTR if interrupted, however
@ -325,7 +326,8 @@ int pipecommon_close(FAR struct file *filep)
/* Check if the decremented inode reference count would go to zero */
if (inode->i_crefs > 1)
dev->d_crefs--;
if (dev->d_crefs > 0)
{
/* More references.. If opened for writing, decrement the count of
* writers on the pipe instance.
@ -886,26 +888,28 @@ int pipecommon_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
int pipecommon_unlink(FAR struct inode *inode)
{
FAR struct pipe_dev_s *dev;
int ret;
DEBUGASSERT(inode->i_private);
dev = inode->i_private;
ret = nxmutex_lock(&dev->d_bflock);
if (ret < 0)
{
return ret;
}
if (dev->d_crefs <= 0)
{
circbuf_uninit(&dev->d_buffer);
pipecommon_freedev(dev);
return OK;
}
/* Mark the pipe unlinked */
PIPE_UNLINK(dev->d_flags);
/* Are the any open references to the driver? */
if (inode->i_crefs == 1)
{
/* No.. free the buffer (if there is one) */
circbuf_uninit(&dev->d_buffer);
/* And free the device structure. */
pipecommon_freedev(dev);
}
nxmutex_unlock(&dev->d_bflock);
return OK;
}

1
drivers/pipes/pipe_common.h
View File

@ -126,6 +126,7 @@ struct pipe_dev_s
uint8_t d_nwriters; /* Number of reference counts for write access */
uint8_t d_nreaders; /* Number of reference counts for read access */
uint8_t d_flags; /* See PIPE_FLAG_* definitions */
int16_t d_crefs; /* References to dev */
struct circbuf_s d_buffer; /* Buffer allocated when device opened */
/* The following is a list if poll structures of threads waiting for