From patchwork Sat May 25 22:35:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adam Richter X-Patchwork-Id: 1105431 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-cifs-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="G4qv4yss"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45BJ4219k6z9s1c for ; Sun, 26 May 2019 08:36:01 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726187AbfEYWf7 (ORCPT ); Sat, 25 May 2019 18:35:59 -0400 Received: from mail-vs1-f47.google.com ([209.85.217.47]:36375 "EHLO mail-vs1-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725989AbfEYWf7 (ORCPT ); Sat, 25 May 2019 18:35:59 -0400 Received: by mail-vs1-f47.google.com with SMTP id l20so8271704vsp.3 for ; Sat, 25 May 2019 15:35:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to; bh=3yXjbWerqXcHUYa+TCJJdoQqGeMhHAiIgNdsitC+Gls=; b=G4qv4yssviRrZ/apbmCTipqLL3KKedhIGQCwXTgRYySin+c8NtEQVXvBphsa+asFLU BBnl1EfuSWgCakpS55FZ0sGfdX5InChgk0QE81mHlvAEIs3kZiKGMEF+RHJCmKXVtmTZ l4NMr826nCEVLI+lJYfBSMlkHLLisV2greejRYmT60n9LoA8yIr8miNVkVyi4A3B5JET Q5OSvow/e+5LLUdKk5RFhIaOECjs2biRZbrrTvFQoURpc/koFNkS6NoY1iATkUsR/ocg 86guIaRlW80ErhMnWUCpXlQJUJWZnTKMBOqDpvKuCGhmBkb88xeyFE/n2w5V/86bYHza 9j9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=3yXjbWerqXcHUYa+TCJJdoQqGeMhHAiIgNdsitC+Gls=; b=sUx2UsRfIeiuOxtUfp+wg4W8oQRGRO1x9z2Rs80US1r4cktlr6oNBGAQkbTWu9JgIE yL1U9lOdbII8o8DnHUZREaF2+Q6+5/6UA5vT6w2q/+hyXzCSWCuKwFLQO3RlFW9EECtb o3dOONLZDfTraNNmm2RXL+B4lgW9zce93YVyY4vFf+uPVhMBP/HOKyWxJ4WDhWdQpTqj eRvOA6wqnDbd+BGAOudAljKnB172Yzqk1+bEOuV5dOEpip/lIxgk4In9947GqbCt+7OV I1RexUaLsrAim9UIcfqEBZ49lX1zCc4WoU0O1PLjz8pUbL7kLJWo7CrT+7iqZ7QvmTQ0 NdAQ== X-Gm-Message-State: APjAAAWnmbjSHCovNk8S/bvhBhc6LIwRlUdG8ON+wAVDIEynG7+ltV6L 4b2qu19pP0KvXbFgMA7+Q71YxTl7YrS/h/8zQCSYjoEpdpg= X-Google-Smtp-Source: APXvYqzgfH7gBUyr1yuiXJJXbew/LxJpjK6taqDT/RiSKp4hJzwVydkp2tAGlnI/Q8PSlsainHo6UW8zcCjYPASAZ3E= X-Received: by 2002:a67:ef45:: with SMTP id k5mr36013915vsr.105.1558823757884; Sat, 25 May 2019 15:35:57 -0700 (PDT) MIME-Version: 1.0 From: Adam Richter Date: Sat, 25 May 2019 15:35:47 -0700 Message-ID: Subject: [PATCH] cifs-utils: smbinfo.c: probably harmless wrong memset sizes + printf format correction To: linux-cifs@vger.kernel.org Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org The attached patch is my attempt at fixing two possibly harmless complaints from "cppcheck --enable=warning" from the cifs-utils git master branch version of smbinfo.c. 1. A printf format should have been "%u" instead of "%d" in print_quota. 2. An incorrect size was being passed to memset in thirteen nearly identical places, each using "sizeof(qi)" when "sizeof(*qi)". I am not sure but I think these mistakes were probably harmless because the memset calls might all be unnecessary and the sizes passed to each memset call might never have been larger than it was supposed to be. Because each of the effected memset calls was immediately preceded by the malloc which allocated the data structure and because they each ignored the possibility that malloc could fail, I made a function, xmalloc0 to combine allocating the memory, zeroing it and exiting with a non-zero exit value and a failure message on allocation failure (which appears to be a fine way to handle the error in this program). The function uses calloc, which could introduce an unnecessary multiply, in the hopes that some calloc implementations may avoid the memset in the case of freshly allocated memory from mmap, which would probably be the case in this program, although I do not know if any calloc implementations make this optimization. Anyhow, at least this way, the size of the data structure is only computed once in the source code. I realize that these memory allocations may all be for small data structures that should be allocated on the stack and also may not need to be cleared to all zeroes, but I did not want to delve into coding style conventions for stack allocation in the CIFS source tree, and I was not 100% certain that clearing the allocated memory was unnecessary, although I do see other lines that explicitly initialize some field in that that allocated memory to zero. So, please feel free to replace my changes with something better or one that involves less code churn. I should also warn that my only testing of these changes was to make sure that "cppcheck --enable=warning" no longer complains, that the file compiled without complaint (with cifs-utils standard "-Wall -Wextra" arguments) and that "./smbinfo quote /dev/null" got past the memory allocation to the (correct) ioctl error for /dev/null. Also, I am not a CIFS developer and this may be the first time I have submitted a patch, certainly the first time I remember, so please forgive me and feel free to instruct me if I should be following some different process to submit this patch. Thanks in advance for considering this patch submission. Adam --- cifs-utils/smbinfo.c.orig 2019-05-25 14:19:56.758474588 -0700 +++ cifs-utils/smbinfo.c 2019-05-25 14:26:22.633256913 -0700 @@ -97,6 +97,18 @@ usage(char *name) exit(1); } +static void * +xmalloc0(size_t nbytes) +{ + void *result = calloc(1, nbytes); + if (result == NULL) { + fprintf(stderr, "%s: failed to allocate %zu bytes.\n", + __func__, nbytes); + exit(1); + } + return result; +} + static void win_to_timeval(uint64_t smb2_time, struct timeval *tv) { @@ -225,8 +237,7 @@ fsctlgetobjid(int f) fstat(f, &st); - qi = malloc(sizeof(struct smb_query_info) + 64); - memset(qi, 0, sizeof(qi) + 64); + qi = xmalloc0(sizeof(struct smb_query_info) + 64); qi->info_type = 0x9009c; qi->file_info_class = 0; qi->additional_information = 0; @@ -268,8 +279,7 @@ fileaccessinfo(int f) fstat(f, &st); - qi = malloc(sizeof(struct smb_query_info) + 4); - memset(qi, 0, sizeof(qi) + 4); + qi = xmalloc0(sizeof(struct smb_query_info) + 4); qi->info_type = 0x01; qi->file_info_class = 8; qi->additional_information = 0; @@ -322,8 +332,7 @@ filealigninfo(int f) { struct smb_query_info *qi; - qi = malloc(sizeof(struct smb_query_info) + 4); - memset(qi, 0, sizeof(qi) + 4); + qi = xmalloc0(sizeof(struct smb_query_info) + 4); qi->info_type = 0x01; qi->file_info_class = 17; qi->additional_information = 0; @@ -392,8 +401,7 @@ filebasicinfo(int f) { struct smb_query_info *qi; - qi = malloc(sizeof(struct smb_query_info) + 40); - memset(qi, 0, sizeof(qi) + 40); + qi = xmalloc0(sizeof(struct smb_query_info) + 40); qi->info_type = 0x01; qi->file_info_class = 4; qi->additional_information = 0; @@ -432,8 +440,7 @@ filestandardinfo(int f) { struct smb_query_info *qi; - qi = malloc(sizeof(struct smb_query_info) + 24); - memset(qi, 0, sizeof(qi) + 24); + qi = xmalloc0(sizeof(struct smb_query_info) + 24); qi->info_type = 0x01; qi->file_info_class = 5; qi->additional_information = 0; @@ -462,8 +469,7 @@ fileinternalinfo(int f) { struct smb_query_info *qi; - qi = malloc(sizeof(struct smb_query_info) + 8); - memset(qi, 0, sizeof(qi) + 8); + qi = xmalloc0(sizeof(struct smb_query_info) + 8); qi->info_type = 0x01; qi->file_info_class = 6; qi->additional_information = 0; @@ -505,8 +511,7 @@ filemodeinfo(int f) { struct smb_query_info *qi; - qi = malloc(sizeof(struct smb_query_info) + 4); - memset(qi, 0, sizeof(qi) + 4); + qi = xmalloc0(sizeof(struct smb_query_info) + 4); qi->info_type = 0x01; qi->file_info_class = 16; qi->additional_information = 0; @@ -535,8 +540,7 @@ filepositioninfo(int f) { struct smb_query_info *qi; - qi = malloc(sizeof(struct smb_query_info) + 8); - memset(qi, 0, sizeof(qi) + 8); + qi = xmalloc0(sizeof(struct smb_query_info) + 8); qi->info_type = 0x01; qi->file_info_class = 14; qi->additional_information = 0; @@ -565,8 +569,7 @@ fileeainfo(int f) { struct smb_query_info *qi; - qi = malloc(sizeof(struct smb_query_info) + 4); - memset(qi, 0, sizeof(qi) + 4); + qi = xmalloc0(sizeof(struct smb_query_info) + 4); qi->info_type = 0x01; qi->file_info_class = 7; qi->additional_information = 0; @@ -610,8 +613,7 @@ filefsfullsizeinfo(int f) { struct smb_query_info *qi; - qi = malloc(sizeof(struct smb_query_info) + 32); - memset(qi, 0, sizeof(qi) + 32); + qi = xmalloc0(sizeof(struct smb_query_info) + 32); qi->info_type = 0x02; qi->file_info_class = 7; qi->additional_information = 0; @@ -634,8 +636,7 @@ fileallinfo(int f) fstat(f, &st); - qi = malloc(sizeof(struct smb_query_info) + INPUT_BUFFER_LENGTH); - memset(qi, 0, sizeof(qi) + INPUT_BUFFER_LENGTH); + qi = xmalloc0(sizeof(struct smb_query_info) + INPUT_BUFFER_LENGTH); qi->info_type = 0x01; qi->file_info_class = 18; qi->additional_information = 0; @@ -862,8 +863,7 @@ secdesc(int f) fstat(f, &st); - qi = malloc(sizeof(struct smb_query_info) + INPUT_BUFFER_LENGTH); - memset(qi, 0, sizeof(qi) + INPUT_BUFFER_LENGTH); + qi = xmalloc0(sizeof(struct smb_query_info) + INPUT_BUFFER_LENGTH); qi->info_type = 0x03; qi->file_info_class = 0; qi->additional_information = 0x00000007; /* Owner, Group, Dacl */ @@ -892,7 +892,7 @@ one_more: memcpy(&u32, &sd[off + 4], 4); u32 = le32toh(u32); - printf("SID Length %d\n", u32); + printf("SID Length %u\n", u32); memcpy(&u64, &sd[off + 8], 8); win_to_timeval(le64toh(u64), &tv); @@ -941,8 +941,7 @@ quota(int f) char *buf; int i; - qi = malloc(sizeof(struct smb_query_info) + INPUT_BUFFER_LENGTH); - memset(qi, 0, sizeof(struct smb_query_info) + INPUT_BUFFER_LENGTH); + qi = xmalloc0(sizeof(struct smb_query_info) + INPUT_BUFFER_LENGTH); qi->info_type = 0x04; qi->file_info_class = 0; qi->additional_information = 0; /* Owner, Group, Dacl */