fs/ncpfs: fix error paths and goto statements in ncp_fill_super()

The label 'out_bdi' should be followed by bdi_destroy() instead of fput() which should be after the 'out_fput' label.

If bdi_setup_and_register() fails then jump to the 'out_fput' label instead of the 'out_bdi' one.

If fget(data.info_fd) fails then jump to the previously fixed 'out_bdi' label to call bdi_destroy() otherwise the bdi object will not be destroyed.

Compile tested only.

---
 fs/ncpfs/inode.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

index 5b5fa33..cbd1a61 100644
@@ -548,7 +548,7 @@ static int ncp_fill_super(struct super_block *sb, void *raw_data, int silent)
 
 	error = bdi_setup_and_register(&server->bdi, "ncpfs", BDI_CAP_MAP_COPY);
 	if (error)
-		goto out_bdi;
+		goto out_fput;
 
 	server->ncp_filp = ncp_filp;
 	server->ncp_sock = sock;

@@ -559,7 +559,7 @@ static int ncp_fill_super(struct super_block *sb, void *raw_data, int silent)
 		error = -EBADF;
 		server->info_filp = fget(data.info_fd);
 		if (!server->info_filp)
-			goto out_fput;
+			goto out_bdi;
 		error = -ENOTSOCK;
 		sock_inode = server->info_filp->f_path.dentry->d_inode;
 		if (!S_ISSOCK(sock_inode->i_mode))

@@ -746,9 +746,9 @@ out_nls:
 out_fput2:
 	if (server->info_filp)
 		fput(server->info_filp);
-out_fput:
-	bdi_destroy(&server->bdi);
 out_bdi:
+	bdi_destroy(&server->bdi);
+out_fput:
 	/* 23/12/1998 Marcin Dalecki <[email protected]>:
 	 * 
 	 * The previously used put_filp(ncp_filp); was bogus, since

1.7.1

[davem Cc'd, since there's an ugly socket-related problem in that area]

Applied, but now that I've looked at that code... It, along with several other places, overwrites a bunch of fields in sock->sk. What happens if some joker feeds that descriptor to, say, svc_addsock()? It does the same kind of thing, and both assume that no bonehead programmer would ever do something that dumb. Moreover, what if somebody feeds the same fd to ncpfs mount or svc_addsock twice?

I'd done a quick grep and AFAICS, most of the places doing that kind of thing is acting on internally created socket, so no such problem for them. Or they take care to check that somebody has already done that to socket in question. Potentially unpleasant ones:
drivers/scsi/iscsi_tcp.c:iscsi_sw_tcp_conn_bind() fs/dlm/lowcomms.c:process_sctp_notification() fs/ncpfs/inode.c:ncp_fill_super() net/sunrpc/svcsock.c:svc_addsock()

I'm not familiar with that area at all; not even enough to tell if locking is wrong in svc_addsock() - I suspect that it is, but...

Dave, am I right assuming that all those places ought to check if somebody is already parasiting on the socket in question and fail with -EBUSY or something like that?