-----Original Message----- From: Timo Aaltonen [mailto:timo.aaltonen@xxxxxxxx] Sent: Wednesday, May 05, 2010 6:34 PM To: Staubach, Peter Cc: timo.aaltonen@xxxxxxxx; kwc@xxxxxxxxxxxxxx; bfields@xxxxxxxxxxxx; linux-nfs@xxxxxxxxxxxxxxx Subject: RE: [PATCH] Check for AD style machine principal name On Thu, 6 May 2010, Staubach_Peter@xxxxxxx wrote: > -----Original Message----- > From: linux-nfs-owner@xxxxxxxxxxxxxxx [mailto:linux-nfs-owner@xxxxxxxxxxxxxxx] On Behalf Of Timo Aaltonen > Sent: Wednesday, May 05, 2010 6:16 PM > To: Kevin Coffman > Cc: J. Bruce Fields; linux-nfs@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] Check for AD style machine principal name > > On Wed, 5 May 2010, Kevin Coffman wrote: > >> On Wed, May 5, 2010 at 12:53 PM, Timo Aaltonen <timo.aaltonen@xxxxxxxx> wrote: >>> >>> CC:ing the "new" list. >>> >>> On Tue, 27 Apr 2010, J. Bruce Fields wrote: >>> >>>> On Mon, Apr 26, 2010 at 05:00:59PM +0300, timo.aaltonen@xxxxxxxx wrote: >>>>> >>>>> @@ -737,6 +737,26 @@ gssd_search_krb5_keytab(krb5_context context, >>>>> krb5_keytab kt, >>>>> } >>>>> >>>>> /* >>>>> + * Convert the hostname to machine principal name as created >>>>> + * by MS Active Directory. >>>>> +*/ >>>>> + >>>>> +static char * >>>>> +hostname_to_adprinc(char *name) >>>>> +{ >>>>> + int i = 0; >>>>> + char *buf = strdup(name); >>>> >>>> We should handle the possible NULL return. >>> >>> How about not calling hostname_to_adprinc from find_keytab_entry unless >>> myhostname is != NULL? >> >> I think he meant that strdup() may fail, and return NULL. That has to >> be handled here, and a possible resulting NULL return from >> host_to_adprinc() should also be handled below. > > Ahh.. ok so how about this: > > static char * > hostname_to_adprinc(char *name) > { > int i = 0; > int len = strlen(name); > char *buf; > if ((buf = malloc(len+1))) { > >> Doesn't this need to be "+2" since you are adding a '$' and a '\0' to the end of the string, neither of which is included in the strlen() calculation? Yep, you're right. Shouldn't adding to the buffer fail then, when the malloc leaves the string short? >> Nope, you just step on memory that potentially belongs to something else, causing corruption. ps > strcpy(buf, name); > while(i < len) { > buf[i] = toupper(buf[i]); > i++; > } > >> Instead of copying name to buf and then converting the characters in buf to uppercase, why not copy and convert in one loop? This buf[i] = toupper(name[i]); works fine, thanks :) -- Timo Aaltonen Systems Specialist IT Services, Aalto University -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
