From patchwork Fri Oct 11 15:12:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 3024911 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id D62FE9F243 for ; Fri, 11 Oct 2013 15:12:48 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AB6E9202C0 for ; Fri, 11 Oct 2013 15:12:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7B755202BE for ; Fri, 11 Oct 2013 15:12:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752198Ab3JKPMq (ORCPT ); Fri, 11 Oct 2013 11:12:46 -0400 Received: from mail-ye0-f169.google.com ([209.85.213.169]:45748 "EHLO mail-ye0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751932Ab3JKPMp (ORCPT ); Fri, 11 Oct 2013 11:12:45 -0400 Received: by mail-ye0-f169.google.com with SMTP id r10so775514yen.14 for ; Fri, 11 Oct 2013 08:12:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:subject:date:message-id :in-reply-to:references; bh=o/Z2i5nFaAoRHbDfnTYq2Yj7W7nb5mGsQ883+rN7Mf8=; b=EZ4B/I6hzN5vJnw9HGC9Gu7eAAPT2ijPd2u1PelF0OUyMZU2HcWEbux4afGIidqwLr +MoH6O+PmjR481+/myjfRtunXKE93tW9EdlQ1t/AdGOE/gPvA5B2FghbX6QP1cxLfDZ5 NG8KBg9xh2pI2mJu4sy3v4kl7AiCAifQFPqKoYpHLCHs66gN5gwd9uda3Dy0CqM0BaHI ptxisukuXZl5/GI3jieCd4KQ0d2nN1sW+yyl9cf79kh+d0qDvaMk8gr2NLzSv9cLp8m+ OFX4ZQ56byKDcGnbXsfJgw+3AvmRMjcTPf4FZ/1phnUprdXF/SYV7k9CkmLbuyjhhtTm T9Zw== X-Gm-Message-State: ALoCoQlqJZL5MYNHOwgemmMXWE2WKxSyMmT1p2JmGKOS5ktQp7v9O4APq2+D54aSvmGBG+3z41jF X-Received: by 10.236.32.3 with SMTP id n3mr12066971yha.25.1381504364679; Fri, 11 Oct 2013 08:12:44 -0700 (PDT) Received: from salusa.poochiereds.net (cpe-107-015-124-230.nc.res.rr.com. [107.15.124.230]) by mx.google.com with ESMTPSA id v45sm79085306yha.2.1969.12.31.16.00.00 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 11 Oct 2013 08:12:44 -0700 (PDT) From: Jeff Layton To: linux-cifs@vger.kernel.org Subject: [PATCH v2][cifs-utils] mount.cifs: fix bad free() of string returned by dirname() Date: Fri, 11 Oct 2013 11:12:40 -0400 Message-Id: <1381504360-10831-1-git-send-email-jlayton@samba.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1381501673-7286-1-git-send-email-jlayton@samba.org> References: <1381501673-7286-1-git-send-email-jlayton@samba.org> Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP v2: - explicitly free mtabtmpfile memory Coverity says: Error: CPPCHECK_WARNING: [#def10] cifs-utils-6.2/mount.cifs.c:1518: error[memleakOnRealloc]: Common realloc mistake: 'mtabdir' nulled but not freed upon failure del_mtab has a number of bugs in handling of allocated memory: a) the return value of strdup() is not checked b) It calls realloc() on a pointer that wasn't returned by an allocation function (e.g. malloc, calloc, etc.) c) If realloc() fails, it doesn't call free() on the original memory returned by strdup() Fix all of these bugs and add newlines to the end of the error messages in del_mtab. Signed-off-by: Jeff Layton --- mount.cifs.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/mount.cifs.c b/mount.cifs.c index 7206dcb..497665d 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -1508,23 +1508,29 @@ add_mtab_exit: static int del_mtab(char *mountpoint) { - int tmprc, rc = 0; + int len, tmprc, rc = 0; FILE *mnttmp, *mntmtab; struct mntent *mountent; - char *mtabfile, *mtabdir, *mtabtmpfile; + char *mtabfile, *mtabdir, *mtabtmpfile = NULL; mtabfile = strdup(MOUNTED); - mtabdir = dirname(mtabfile); - mtabdir = realloc(mtabdir, strlen(mtabdir) + strlen(MNT_TMP_FILE) + 2); - if (!mtabdir) { - fprintf(stderr, "del_mtab: cannot determine current mtab path"); + if (!mtabfile) { + fprintf(stderr, "del_mtab: cannot strdup MOUNTED\n"); rc = EX_FILEIO; goto del_mtab_exit; } - mtabtmpfile = strcat(mtabdir, MNT_TMP_FILE); + mtabdir = dirname(mtabfile); + len = strlen(mtabdir) + strlen(MNT_TMP_FILE); + mtabtmpfile = malloc(len + 1); if (!mtabtmpfile) { - fprintf(stderr, "del_mtab: cannot allocate memory to tmp file"); + fprintf(stderr, "del_mtab: cannot allocate memory to tmp file\n"); + rc = EX_FILEIO; + goto del_mtab_exit; + } + + if (sprintf(mtabtmpfile, "%s%s", mtabdir, MNT_TMP_FILE) != len) { + fprintf(stderr, "del_mtab: error writing new string\n"); rc = EX_FILEIO; goto del_mtab_exit; } @@ -1532,14 +1538,14 @@ del_mtab(char *mountpoint) atexit(unlock_mtab); rc = lock_mtab(); if (rc) { - fprintf(stderr, "del_mtab: cannot lock mtab"); + fprintf(stderr, "del_mtab: cannot lock mtab\n"); rc = EX_FILEIO; goto del_mtab_exit; } mtabtmpfile = mktemp(mtabtmpfile); if (!mtabtmpfile) { - fprintf(stderr, "del_mtab: cannot setup tmp file destination"); + fprintf(stderr, "del_mtab: cannot setup tmp file destination\n"); rc = EX_FILEIO; goto del_mtab_exit; } @@ -1587,7 +1593,8 @@ del_mtab(char *mountpoint) del_mtab_exit: unlock_mtab(); - free(mtabdir); + free(mtabtmpfile); + free(mtabfile); return rc; del_mtab_error: