[Mono-devel-list] updated gac patches

[Paolo Molaro]

On 04/01/04 Jackson Harper wrote:
> --- metadata/appdomain.c	28 Mar 2004 16:15:17 -0000	1.122
> +++ metadata/appdomain.c	1 Apr 2004 11:35:16 -0000
> @@ -751,6 +751,7 @@
>  	g_free ((void *) aname->name);
>  	g_free ((void *) aname->culture);
>  	g_free ((void *) aname->hash_value);
> +	g_free ((void *) aname->public_tok_value);
[...]
> +				aname->public_tok_value = g_strstrip (g_strdup (value));

g_strstrip () can return a pointer that is not the beginning of the
alloced area, so calling g_free () on it later will make it crash (bug
was there before as well). Also I'm not sure what the code did before
here and why we don't need it anymore. Care to explain?

> --- metadata/assembly.c	1 Apr 2004 09:12:05 -0000	1.86
> +++ metadata/assembly.c	1 Apr 2004 11:35:16 -0000
> @@ -22,8 +22,11 @@
[...]
> +gboolean allow_user_gac = FALSE;

Make static.

> +static gchar*
> +encode_public_tok (const guchar *token, gint32 len)
> +{
> +	static gchar allowed [] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
> +	gchar *res;
> +	int i;
> +
> +	res = g_malloc (len * 2);
> +	for (i = 0; i < len; i++) {
> +		res [i * 2] = allowed [token [i] >> 4];
> +		res [i * 2 + 1] = allowed [token [i] & 0xF];
> +	}
> +	res [len * 2] = 0;

Classic buffer overflow with malloced C strings. You need to alloc one
more byte.

> @@ -615,12 +662,12 @@
>  mono_assembly_load (MonoAssemblyName *aname, const char *basedir, MonoImageOpenStatus *status)
>  {
>  	MonoAssembly *result;
> -	char *fullpath, *filename;
> +	char *fullpath, *filename, *asmbname;
> +	int len;
>  
>  	result = invoke_assembly_preload_hook (aname, assemblies_path);
>  	if (result)
>  		return result;
> -
>  	/* g_print ("loading %s\n", aname->name); */
>  	/* special case corlib */
>  	if ((strcmp (aname->name, "mscorlib") == 0) || (strcmp (aname->name, "corlib") == 0)) {
> @@ -645,29 +692,71 @@
>  	result = search_loaded (aname);
>  	if (result)
>  		return result;
> +
>  	/* g_print ("%s not found in cache\n", aname->name); */
> -	if (strstr (aname->name, ".dll"))
> +	if (strstr (aname->name, ".dll")) {
>  		filename = g_strdup (aname->name);
> -	else
> +		len = strlen (filename) - 4;
> +		asmbname = malloc (len);
> +		strncpy (asmbname, aname->name, len);
> +	} else {
>  		filename = g_strconcat (aname->name, ".dll", NULL);
> +		asmbname = g_strdup (aname->name);
> +	}
>  	if (basedir) {
>  		fullpath = g_build_filename (basedir, filename, NULL);
>  		result = mono_assembly_open (fullpath, status);
>  		g_free (fullpath);
>  		if (result) {
> +			result->in_gac = FALSE;
> +			g_free (filename);
> +			g_free (asmbname);
> +			return result;
> +		}
> +	}
> +	/* try the gac */
> +	if (aname->public_tok_value) {

I think the rule in the MS runtime is to look in the GAC first, before
checking in the dir where the referencing assembly is located.

> +		gchar *version;
> +		version = g_strdup_printf ("%d.%d.%d.%d_%s_%s", aname->major,
> +				aname->minor, aname->build, aname->revision,
> +				aname->culture, aname->public_tok_value);
> +
> +		fullpath = g_build_path (G_DIR_SEPARATOR_S, MONO_ASSEMBLIES, "mono", "gac",
> +				asmbname, version, filename, NULL);
> +		result = mono_assembly_open (fullpath, status);
> +
> +		g_free (fullpath);
> +
> +		if (!result && allow_user_gac && getenv ("HOME")) {
> +			fullpath = g_build_path (G_DIR_SEPARATOR_S, getenv ("HOME"), ".mono", "gac",
> +					asmbname, version, filename, NULL);
> +			result = mono_assembly_open (fullpath, status);
> +			g_free (fullpath);
> +		}

It might be worth to have two helper functions (or one) to just do the
lookup in the GAC (ie pull this code outside this function). Also, in
mono we use the GLib function g_get_home_dir () instead of getenv ("HOME").

Thanks.

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better