From patchwork Tue Apr 20 13:34:53 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Scott Lovenberg X-Patchwork-Id: 50541 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.samba.org (fn.samba.org [216.83.154.106]) by ozlabs.org (Postfix) with ESMTP id 250FBB7C33 for ; Tue, 20 Apr 2010 23:35:11 +1000 (EST) Received: from fn.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 928B1AD134; Tue, 20 Apr 2010 07:35:10 -0600 (MDT) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on fn.samba.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=3.8 tests=AWL, BAYES_00, NO_MORE_FUNN, SPF_PASS autolearn=no version=3.2.5 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from mail-vw0-f41.google.com (mail-vw0-f41.google.com [209.85.212.41]) by lists.samba.org (Postfix) with ESMTP id 5B156AD0F5 for ; Tue, 20 Apr 2010 07:35:04 -0600 (MDT) Received: by vws17 with SMTP id 17so988389vws.14 for ; Tue, 20 Apr 2010 06:35:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:reply-to :user-agent:mime-version:to:subject:references:in-reply-to :content-type; bh=lh0LegMobayjEzTLdlQBzNI8oYGHUZpZ0IHZpRjtn0Q=; b=qFlRO9/wVq1gfWwTnbuVskMcwgIxaQW4JNSCvMM5U8tef6s9jRo4wYDMx4xpd0fxKG eAzDnFN7dO+k4qe/mdbJnYmXjCQ/SFQEg1h7ffiN+jnyJTdPK/uOgT8vutEupx0SDHk7 VIIawHhN5Ul1W0Qzhp9j/wHOu113vwFQn4/Sk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:reply-to:user-agent:mime-version:to:subject :references:in-reply-to:content-type; b=oQgHqbsZO038Tq48dH0aaUYpBXRa2nnidi9Huj4WwuVbBY0nboaHecXZhOoRylnpFG 9TbkicDr16NWsN4UMI9bwx8K9bbg5KUNyIFOW0BwKxNs/1CS93O6hpwl5qPYLgLcUDLf iy9HKDR/aUTx73Xu9IxDtN7XtkwjtfivjKgoY= Received: by 10.220.127.22 with SMTP id e22mr4669007vcs.83.1271770502433; Tue, 20 Apr 2010 06:35:02 -0700 (PDT) Received: from secondaryDesktop.myroom.net (24.115.161.116.res-cmts.flt.ptd.net [24.115.161.116]) by mx.google.com with ESMTPS id b22sm23760579vcp.20.2010.04.20.06.34.54 (version=SSLv3 cipher=RC4-MD5); Tue, 20 Apr 2010 06:34:54 -0700 (PDT) Message-ID: <4BCDAD7D.2080201@gmail.com> Date: Tue, 20 Apr 2010 09:34:53 -0400 From: Scott Lovenberg User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.9) Gecko/20100330 Fedora/3.0.4-1.fc11 Lightning/1.0b2pre Thunderbird/3.0.4 MIME-Version: 1.0 To: linux-cifs-client@lists.samba.org References: <20100418072602.181b43d6@tlielax.poochiereds.net> In-Reply-To: <20100418072602.181b43d6@tlielax.poochiereds.net> Subject: Re: [linux-cifs-client] [Patch] Clean up open_cred_file() X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: scott.lovenberg@gmail.com, linux-cifs-client@lists.samba.org List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces@lists.samba.org Errors-To: linux-cifs-client-bounces@lists.samba.org [Sent this to Jeff a bit ago and forgot to CC the mailing list - Scott] Jeff, I agree on all of your points. Good catch on the second memset(), too. The patch below (and attached) has the changes you mentioned. Also, for the sake of consistency and to keep the indentation sane, I've switched to the 'if(!temp_val) continue;' flow that was used for parsing one case but not the other two. indicate @@ -128,7 +128,7 @@ struct parsed_mount_info { char share[MAX_SHARE_LEN + 1]; char prefix[PATH_MAX + 1]; char options[MAX_OPTIONS_LEN]; - char domain[DOMAIN_SIZE + 1]; + char domain[MAX_DOMAIN_SIZE + 1]; char username[MAX_USERNAME_SIZE + 1]; char password[MOUNT_PASSWD_SIZE + 1]; char addrlist[MAX_ADDR_LIST_LEN]; @@ -511,14 +511,26 @@ toggle_dac_capability(int writable, int enable) #endif /* HAVE_LIBCAP */ #endif /* HAVE_LIBCAP_NG */ +/* + * Null terminates string at first '\n'. + */ +static void null_terminate_endl(char* source) +{ + char* newline = strchr(source, '\n'); + if (newline) + *newline = '\0'; +} + static int open_cred_file(char *file_name, struct parsed_mount_info *parsed_info) { char *line_buf; - char *temp_val, *newline; + char *temp_val; FILE *fs = NULL; - int i, length; - + int i; + const int line_buf_size = 4096; + const int min_non_white = 10; + i = toggle_dac_capability(0, 1); if (i) return i; @@ -541,86 +553,59 @@ static int open_cred_file(char *file_name, return i; } - line_buf = (char *)malloc(4096); + line_buf = (char *)malloc(line_buf_size); if (line_buf == NULL) { fclose(fs); return EX_SYSERR; } - while (fgets(line_buf, 4096, fs)) { - /* parse line from credential file */ - + /* parse line from credential file*/ + while (fgets(line_buf, line_buf_size, fs)) { /* eat leading white space */ - for (i = 0; i < 4086; i++) { + for (i = 0; i < line_buf_size - min_non_white + 1; i++) { if ((line_buf[i] != ' ') && (line_buf[i] != '\t')) break; - /* if whitespace - skip past it */ } + null_terminate_endl(line_buf); - /* NULL terminate at newline */ - newline = strchr(line_buf + i, '\n'); - if (newline) - *newline = '\0'; - + /* parse user */ if (strncasecmp("user", line_buf + i, 4) == 0) { temp_val = strchr(line_buf + i, '='); - if (temp_val) { - /* go past equals sign */ - temp_val++; - for (length = 0; length < 4087; length++) { - if ((temp_val[length] == '\n') - || (temp_val[length] == '\0')) { - temp_val[length] = '\0'; - break; - } - } - if (length > 4086) { - fprintf(stderr, - "mount.cifs failed due to malformed username in credentials file\n"); - memset(line_buf, 0, 4096); - return EX_USAGE; - } - parsed_info->got_user = 1; - strlcpy(parsed_info->username, temp_val, - sizeof(parsed_info->username)); - } - } else if (strncasecmp("pass", line_buf + i, 4) == 0) { + if (!temp_val) + continue; + /* go past equals sign */ + temp_val++; + parse_username(temp_val, parsed_info); + memset(line_buf, 0, line_buf_size); + } + + /* parse password */ + else if (strncasecmp("pass", line_buf + i, 4) == 0) { temp_val = strchr(line_buf + i, '='); if (!temp_val) continue; - ++temp_val; + /* go past equals sign */ + temp_val++; i = set_password(parsed_info, temp_val); + /* zero out password from buffer */ + memset(line_buf, 0, line_buf_size); if (i) return i; - } else if (strncasecmp("dom", line_buf + i, 3) == 0) { + } + + /* parse domain */ + else if (strncasecmp("dom", line_buf + i, 3) == 0) { temp_val = strchr(line_buf + i, '='); - if (temp_val) { - /* go past equals sign */ - temp_val++; - if (parsed_info->verboseflag) - fprintf(stderr, "\nDomain %s\n", - temp_val); - - for (length = 0; length < DOMAIN_SIZE + 1; - length++) { - if ((temp_val[length] == '\n') - || (temp_val[length] == '\0')) { - temp_val[length] = '\0'; - break; - } - } - - if (length > DOMAIN_SIZE) { - fprintf(stderr, - "mount.cifs failed: domain in credentials file too long\n"); - return EX_USAGE; - } - - strlcpy(parsed_info->domain, temp_val, - sizeof(parsed_info->domain)); + if (!temp_val) + continue; + /* go past equals sign */ + temp_val++; + if (parsed_info->verboseflag) + fprintf(stderr, "\nDomain %s\n", temp_val); + strlcpy(parsed_info->domain, temp_val, + sizeof(parsed_info->domain)); } } - } fclose(fs); SAFE_FREE(line_buf); diff --git a/mount.cifs.c b/mount.cifs.c index 1aa3329..98e5665 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -98,7 +98,7 @@ #endif #define MOUNT_PASSWD_SIZE 128 -#define DOMAIN_SIZE 64 +#define MAX_DOMAIN_SIZE 64 /* * value of the ver= option that gets passed to the kernel. Used to indicate @@ -128,7 +128,7 @@ struct parsed_mount_info { char share[MAX_SHARE_LEN + 1]; char prefix[PATH_MAX + 1]; char options[MAX_OPTIONS_LEN]; - char domain[DOMAIN_SIZE + 1]; + char domain[MAX_DOMAIN_SIZE + 1]; char username[MAX_USERNAME_SIZE + 1]; char password[MOUNT_PASSWD_SIZE + 1]; char addrlist[MAX_ADDR_LIST_LEN]; @@ -511,14 +511,26 @@ toggle_dac_capability(int writable, int enable) #endif /* HAVE_LIBCAP */ #endif /* HAVE_LIBCAP_NG */ +/* + * Null terminates string at first '\n'. + */ +static void null_terminate_endl(char* source) +{ + char* newline = strchr(source, '\n'); + if (newline) + *newline = '\0'; +} + static int open_cred_file(char *file_name, struct parsed_mount_info *parsed_info) { char *line_buf; - char *temp_val, *newline; + char *temp_val; FILE *fs = NULL; - int i, length; - + int i; + const int line_buf_size = 4096; + const int min_non_white = 10; + i = toggle_dac_capability(0, 1); if (i) return i; @@ -541,86 +553,59 @@ static int open_cred_file(char *file_name, return i; } - line_buf = (char *)malloc(4096); + line_buf = (char *)malloc(line_buf_size); if (line_buf == NULL) { fclose(fs); return EX_SYSERR; } - while (fgets(line_buf, 4096, fs)) { - /* parse line from credential file */ - + /* parse line from credential file*/ + while (fgets(line_buf, line_buf_size, fs)) { /* eat leading white space */ - for (i = 0; i < 4086; i++) { + for (i = 0; i < line_buf_size - min_non_white + 1; i++) { if ((line_buf[i] != ' ') && (line_buf[i] != '\t')) break; - /* if whitespace - skip past it */ } + null_terminate_endl(line_buf); - /* NULL terminate at newline */ - newline = strchr(line_buf + i, '\n'); - if (newline) - *newline = '\0'; - + /* parse user */ if (strncasecmp("user", line_buf + i, 4) == 0) { temp_val = strchr(line_buf + i, '='); - if (temp_val) { - /* go past equals sign */ - temp_val++; - for (length = 0; length < 4087; length++) { - if ((temp_val[length] == '\n') - || (temp_val[length] == '\0')) { - temp_val[length] = '\0'; - break; - } - } - if (length > 4086) { - fprintf(stderr, - "mount.cifs failed due to malformed username in credentials file\n"); - memset(line_buf, 0, 4096); - return EX_USAGE; - } - parsed_info->got_user = 1; - strlcpy(parsed_info->username, temp_val, - sizeof(parsed_info->username)); - } - } else if (strncasecmp("pass", line_buf + i, 4) == 0) { + if (!temp_val) + continue; + /* go past equals sign */ + temp_val++; + parse_username(temp_val, parsed_info); + memset(line_buf, 0, line_buf_size); + } + + /* parse password */ + else if (strncasecmp("pass", line_buf + i, 4) == 0) { temp_val = strchr(line_buf + i, '='); if (!temp_val) continue; - ++temp_val; + /* go past equals sign */ + temp_val++; i = set_password(parsed_info, temp_val); + /* zero out password from buffer */ + memset(line_buf, 0, line_buf_size); if (i) return i; - } else if (strncasecmp("dom", line_buf + i, 3) == 0) { + } + + /* parse domain */ + else if (strncasecmp("dom", line_buf + i, 3) == 0) { temp_val = strchr(line_buf + i, '='); - if (temp_val) { - /* go past equals sign */ - temp_val++; - if (parsed_info->verboseflag) - fprintf(stderr, "\nDomain %s\n", - temp_val); - - for (length = 0; length < DOMAIN_SIZE + 1; - length++) { - if ((temp_val[length] == '\n') - || (temp_val[length] == '\0')) { - temp_val[length] = '\0'; - break; - } - } - - if (length > DOMAIN_SIZE) { - fprintf(stderr, - "mount.cifs failed: domain in credentials file too long\n"); - return EX_USAGE; - } - - strlcpy(parsed_info->domain, temp_val, - sizeof(parsed_info->domain)); + if (!temp_val) + continue; + /* go past equals sign */ + temp_val++; + if (parsed_info->verboseflag) + fprintf(stderr, "\nDomain %s\n", temp_val); + strlcpy(parsed_info->domain, temp_val, + sizeof(parsed_info->domain)); } } - } fclose(fs); SAFE_FREE(line_buf);