Re: Patch to fix SMB browsing bugs

[Alexander Larsson <alexl redhat com>]

On Wed, 2004-11-17 at 21:11 +0000, Nielsen wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Attached is a patch which fixes a number of SMB browsing bugs. Haven't
> hacked gnome-vfs before, so I hope this is the right place to post this.
> The patch allows authentication of IPC$ and the master browser which
> allows proper browsing on a domain.

So, what is the exact issue here. You have to authenticate to the master
browser when enumerating the workgroup? What happens if you authenticate
as a guest? Do you actually have to authenticate, or does passing the
username/domain suffice? I was hoping we could avoid getting spammed
with new login dialogs each time you click when navigating workgroups
and smb shares.

I don't like the way your patch handles things. The list of workgroups
is a global cache that is needed to quickly tell the type of a smb uri.
It makes no sense to update the cache "for a uri", if its per-uri then
its not a global cache. Furthermore, setting current_uri to the server
you're browsing, and then calling update_workgroup_cache() which
readdirs smb:// will result in many strange things. It will actually
contact the master browser, but e.g add_cached_server will add the
SmbDefaultUser to the browsed server instead of the master browser. It
will also ask for (and save) the password to the server when it really
needs the password for the master browser.

If we really have to authenticate against the master browser we should
probably handle that specifically in the code. It should have an auth
dialog different from the one for the server you're conencting to. We
should save the master browser user/domain/password for later cache
updates, so we don't keep re-asking for it at random points. Of course,
this makes it hard to correct later if you initially type in the wrong
username/password...

-	in_args.port = ((GnomeVFSToplevelURI *)current_uri)->host_port;
+	in_args.port = 0;

Why this change?

Also, the indentation in patch was weird in places, like you used to few
spaces for tabs or something.

> There are still some other issues.
> 
> There seems to be a deadlock or race in the authentication code. In many
> cases the gnome_vfs_module_callback_invoke call in invoke_fill_auth
> (modules/smb-method.c) never returns.
> 
> When stracing or debugging the process (ie: gnome-vfs-daemon or
> nautilus) all works reliably and as expected, so I suspect a
> locking/race issue. Any ideas what this could could be?

Ugh. I think i know. auth_fn() is called from libsmb when it needs
authentication. This means we've called LOCK_SMB() to protect (thread-
unsafe) libsmbclient. However, auth_fn then calls out various password
related callbacks while still holding the lock. At this time the client
getting the callback can be blocking on LOCK_SMB in a gnome-vfs call,
and will not process the callback, leading to a deadlock.

I really have no idea how to fix this. The way authentication is set up
in libsmbclient is really targeted at a blocking console app. We can't
drop the lock while we do the callback, because there is no guarantee
that libsmb can handle another thread doing things while in authfn.
There is also no way we could return FAIL through libsmbclient so we can
sleep without having libsmbclient active, because if we return something
wrong from authfn libsmbclient automatically tries to do an anon login.

Sigh. I wish some real samba developers would be interested in gnome-
vfs. I think it requires libsmbclient work.

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a world-famous soccer-playing filmmaker who hangs with the wrong crowd. 
She's an elegant snooty femme fatale living homeless in New York's sewers. 
They fight crime!