• Dan Carpenter's avatar
    cifsd: fix error handling in ksmbd_server_init() · 849f59e1
    Dan Carpenter authored
    The error handling in ksmbd_server_init() uses "one function to free
    everything style" which is impossible to audit and leads to several
    canonical bugs.  When we free something that wasn't allocated it may be
    uninitialized, an error pointer, freed in a different function or we
    try freeing "foo->bar" when "foo" is a NULL pointer.  And since the
    code is impossible to audit then it leads to memory leaks.
    
    In the ksmbd_server_init() function, every goto will lead to a crash
    because we have not allocated the work queue but we call
    ksmbd_workqueue_destroy() which tries to flush a NULL work queue.
    Another bug is if ksmbd_init_buffer_pools() fails then it leads to a
    double free because we free "work_cache" twice.  A third type of bug is
    that we forgot to call ksmbd_release_inode_hash() so that is a resource
    leak.
    
    A better way to write error handling is for every function to clean up
    after itself and never leave things partially allocated.  Then we can
    use "free the last successfully allocated resource" style.  That way
    when someone is reading the code they can just track the last resource
    in their head and verify that the goto matches what they expect.
    
    In this patch I modified ksmbd_ipc_init() to clean up after itself and
    then I converted ksmbd_server_init() to use gotos to clean up.
    
    Fixes: cabcebc31de4 ("cifsd: introduce SMB3 kernel server")
    Signed-off-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
    Signed-off-by: default avatarNamjae Jeon <namjae.jeon@samsung.com>
    Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
    849f59e1
vfs_cache.h 5.09 KB