On Sun, May 03, 2009 at 08:01:36PM +0100, Al Viro wrote: > General rules: > * descriptor you've generated is fit only for return to userland; > * descriptor you've got from userland is fit only for *single* > fget() or equivalent, unless you are one of the core syscalls manipulating > the descriptor table itself (dup2, etc.) > * once file is installed in descriptor table, you'd better be past > the last failure exit; sys_close() on cleanup path is not acceptable. > That's what reserving descriptors is for. > > IOW, the sane solution would be to export something that returns your > struct file *. And stop playing with fd completely. Speaking of which, quick look through fget() callers shows this turd: static int p9_socket_open(struct p9_client *client, struct socket *csocket) { int fd, ret; fd = sock_map_fd(csocket, 0); ..... ret = p9_fd_open(client, fd, fd); if (ret < 0) { P9_EPRINTK(KERN_ERR, "p9_socket_open: failed to open fd\n"); sockfd_put(csocket); return ret; } ..... return 0; } where p9_fd_open() calls fget() on its 2nd and 3rd arguments. Which does worse than just a leak, AFAICT - on failure exit it leaves a dangling pointer from descriptor table. On the almost unrelated note, we have (in drivers/sneak_in^Wstaging/usbip) sockfd_to_socket(), with all callers leaking struct file, AFAICS. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html