From patchwork Sun Apr 18 07:57:31 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Scott Lovenberg X-Patchwork-Id: 50390 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 60B33B7D0B for ; Sun, 18 Apr 2010 17:57:39 +1000 (EST) Received: from fn.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 308E4AD075; Sun, 18 Apr 2010 01:57:41 -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=-2.6 required=3.8 tests=BAYES_00,HTML_MESSAGE, 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-gw0-f41.google.com (mail-gw0-f41.google.com [74.125.83.41]) by lists.samba.org (Postfix) with ESMTP id 24AF7ACFC7 for ; Sun, 18 Apr 2010 01:57:34 -0600 (MDT) Received: by gwb15 with SMTP id 15so2106991gwb.14 for ; Sun, 18 Apr 2010 00:57:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:date:received:message-id :subject:from:to:content-type; bh=Yk0l6htc43Ku422Qdu2L5HrQDVz3+tCHZTnhoQhMAAo=; b=nJ9ltjdcrZcdRfzscWufXqZ7RQmf/Lnr7WIKhwnVyCZZZgMN95fAuL0hACCSvyNvWo eDWw/yjI5SskuzWYJNn8Io7M178F9tpaGSkbfkbTWWHbSlB3AXr/ItefcRwud2Spo2Sj ZP3CNDXg1JwgBrR/dk9N6ogNO+XkQYQ4X3aM0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:date:message-id:subject:from:to:content-type; b=SpSka71NVa3+HC5VSrZQ7bN9I3gUUymiJuTEYF47RRnnsfBbDasLnkMlpnUdi73+Po xXLYv7ZFTw5IPsoIgyy/EvMsqSHrbKe3/K0PzgGA5uarQ5UwUjHo9Y7+D7mDWDvBgA+x LBTfsx1ArvwGrmbOazEznAj1f2jsEhsvY0G0Y= MIME-Version: 1.0 Received: by 10.231.34.70 with HTTP; Sun, 18 Apr 2010 00:57:31 -0700 (PDT) Date: Sun, 18 Apr 2010 03:57:31 -0400 Received: by 10.150.56.32 with SMTP id e32mr4142716yba.127.1271577451494; Sun, 18 Apr 2010 00:57:31 -0700 (PDT) Message-ID: From: Scott Lovenberg To: linux-cifs-client@lists.samba.org Subject: [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 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 This patch cleans up the open_cred_file() function in mount.cifs.c. It also zeros out the password buffer. I made the buffer length a static int since macro'ing it seemed like leaky abstraction (it only makes sense within the scope of the function, IMHO). Is this acceptable? P.S. - I'm still getting used to git, please let me know if this patch is formatted incorrectly. diff --git a/mount.cifs.c b/mount.cifs.c index 1aa3329..54feeaf 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,58 +553,54 @@ 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 at newline */ - newline = strchr(line_buf + i, '\n'); - if (newline) - *newline = '\0'; - + /* parse user */ + null_terminate_endl(line_buf); 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) { + null_terminate_endl(temp_val); + if (strlen(temp_val) > MAX_USERNAME_SIZE) { fprintf(stderr, "mount.cifs failed due to malformed username in credentials file\n"); - memset(line_buf, 0, 4096); + memset(line_buf, 0, line_buf_size); return EX_USAGE; } - parsed_info->got_user = 1; - strlcpy(parsed_info->username, temp_val, - sizeof(parsed_info->username)); + parse_username(temp_val, parsed_info); } - } else if (strncasecmp("pass", line_buf + i, 4) == 0) { + } + + /* 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 */ @@ -601,16 +609,8 @@ static int open_cred_file(char *file_name, 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) { + null_terminate_endl(temp_val); + if (strlen(temp_val) > MAX_DOMAIN_SIZE) { fprintf(stderr, "mount.cifs failed: domain in credentials file too long\n"); return EX_USAGE; @@ -620,7 +620,6 @@ static int open_cred_file(char *file_name, sizeof(parsed_info->domain)); } } - } fclose(fs); SAFE_FREE(line_buf);