All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Dmitry Antipov <dmantipov@yandex.ru>,
	Namjae Jeon <linkinjeon@kernel.org>,
	Steve French <stfrench@microsoft.com>,
	Sasha Levin <sashal@kernel.org>,
	sfrench@samba.org, linux-cifs@vger.kernel.org,
	samba-technical@lists.samba.org
Subject: [PATCH AUTOSEL 6.6 29/32] smb: client, common: fix fortify warnings
Date: Mon,  4 Dec 2023 15:32:49 -0500	[thread overview]
Message-ID: <20231204203317.2092321-29-sashal@kernel.org> (raw)
In-Reply-To: <20231204203317.2092321-1-sashal@kernel.org>

From: Dmitry Antipov <dmantipov@yandex.ru>

[ Upstream commit 0015eb6e12384ff1c589928e84deac2ad1ceb236 ]

When compiling with gcc version 14.0.0 20231126 (experimental)
and CONFIG_FORTIFY_SOURCE=y, I've noticed the following:

In file included from ./include/linux/string.h:295,
                 from ./include/linux/bitmap.h:12,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/x86/include/asm/paravirt.h:17,
                 from ./arch/x86/include/asm/cpuid.h:62,
                 from ./arch/x86/include/asm/processor.h:19,
                 from ./arch/x86/include/asm/cpufeature.h:5,
                 from ./arch/x86/include/asm/thread_info.h:53,
                 from ./include/linux/thread_info.h:60,
                 from ./arch/x86/include/asm/preempt.h:9,
                 from ./include/linux/preempt.h:79,
                 from ./include/linux/spinlock.h:56,
                 from ./include/linux/wait.h:9,
                 from ./include/linux/wait_bit.h:8,
                 from ./include/linux/fs.h:6,
                 from fs/smb/client/smb2pdu.c:18:
In function 'fortify_memcpy_chk',
    inlined from '__SMB2_close' at fs/smb/client/smb2pdu.c:3480:4:
./include/linux/fortify-string.h:588:25: warning: call to '__read_overflow2_field'
declared with attribute warning: detected read beyond size of field (2nd parameter);
maybe use struct_group()? [-Wattribute-warning]
  588 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and:

In file included from ./include/linux/string.h:295,
                 from ./include/linux/bitmap.h:12,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/x86/include/asm/paravirt.h:17,
                 from ./arch/x86/include/asm/cpuid.h:62,
                 from ./arch/x86/include/asm/processor.h:19,
                 from ./arch/x86/include/asm/cpufeature.h:5,
                 from ./arch/x86/include/asm/thread_info.h:53,
                 from ./include/linux/thread_info.h:60,
                 from ./arch/x86/include/asm/preempt.h:9,
                 from ./include/linux/preempt.h:79,
                 from ./include/linux/spinlock.h:56,
                 from ./include/linux/wait.h:9,
                 from ./include/linux/wait_bit.h:8,
                 from ./include/linux/fs.h:6,
                 from fs/smb/client/cifssmb.c:17:
In function 'fortify_memcpy_chk',
    inlined from 'CIFS_open' at fs/smb/client/cifssmb.c:1248:3:
./include/linux/fortify-string.h:588:25: warning: call to '__read_overflow2_field'
declared with attribute warning: detected read beyond size of field (2nd parameter);
maybe use struct_group()? [-Wattribute-warning]
  588 |                         __read_overflow2_field(q_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In both cases, the fortification logic inteprets calls to 'memcpy()' as an
attempts to copy an amount of data which exceeds the size of the specified
field (i.e. more than 8 bytes from __le64 value) and thus issues an overread
warning. Both of these warnings may be silenced by using the convenient
'struct_group()' quirk.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/smb/client/cifspdu.h | 24 ++++++++++++++----------
 fs/smb/client/cifssmb.c |  6 ++++--
 fs/smb/client/smb2pdu.c |  8 +++-----
 fs/smb/client/smb2pdu.h | 16 +++++++++-------
 fs/smb/common/smb2pdu.h | 17 ++++++++++-------
 5 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
index a75220db5c1e1..11081492d9436 100644
--- a/fs/smb/client/cifspdu.h
+++ b/fs/smb/client/cifspdu.h
@@ -882,11 +882,13 @@ typedef struct smb_com_open_rsp {
 	__u8 OplockLevel;
 	__u16 Fid;
 	__le32 CreateAction;
-	__le64 CreationTime;
-	__le64 LastAccessTime;
-	__le64 LastWriteTime;
-	__le64 ChangeTime;
-	__le32 FileAttributes;
+	struct_group(common_attributes,
+		__le64 CreationTime;
+		__le64 LastAccessTime;
+		__le64 LastWriteTime;
+		__le64 ChangeTime;
+		__le32 FileAttributes;
+	);
 	__le64 AllocationSize;
 	__le64 EndOfFile;
 	__le16 FileType;
@@ -2264,11 +2266,13 @@ typedef struct {
 /* QueryFileInfo/QueryPathinfo (also for SetPath/SetFile) data buffer formats */
 /******************************************************************************/
 typedef struct { /* data block encoding of response to level 263 QPathInfo */
-	__le64 CreationTime;
-	__le64 LastAccessTime;
-	__le64 LastWriteTime;
-	__le64 ChangeTime;
-	__le32 Attributes;
+	struct_group(common_attributes,
+		__le64 CreationTime;
+		__le64 LastAccessTime;
+		__le64 LastWriteTime;
+		__le64 ChangeTime;
+		__le32 Attributes;
+	);
 	__u32 Pad1;
 	__le64 AllocationSize;
 	__le64 EndOfFile;	/* size ie offset to first free byte in file */
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 25503f1a4fd21..09e66ceb38a25 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1244,8 +1244,10 @@ CIFS_open(const unsigned int xid, struct cifs_open_parms *oparms, int *oplock,
 		*oplock |= CIFS_CREATE_ACTION;
 
 	if (buf) {
-		/* copy from CreationTime to Attributes */
-		memcpy((char *)buf, (char *)&rsp->CreationTime, 36);
+		/* copy commonly used attributes */
+		memcpy(&buf->common_attributes,
+		       &rsp->common_attributes,
+		       sizeof(buf->common_attributes));
 		/* the file_info buf is endian converted by caller */
 		buf->AllocationSize = rsp->AllocationSize;
 		buf->EndOfFile = rsp->EndOfFile;
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index c75a80bb6d9ee..3e388c6cb5c25 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3377,12 +3377,10 @@ __SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 	} else {
 		trace_smb3_close_done(xid, persistent_fid, tcon->tid,
 				      ses->Suid);
-		/*
-		 * Note that have to subtract 4 since struct network_open_info
-		 * has a final 4 byte pad that close response does not have
-		 */
 		if (pbuf)
-			memcpy(pbuf, (char *)&rsp->CreationTime, sizeof(*pbuf) - 4);
+			memcpy(&pbuf->network_open_info,
+			       &rsp->network_open_info,
+			       sizeof(pbuf->network_open_info));
 	}
 
 	atomic_dec(&tcon->num_remote_opens);
diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h
index 220994d0a0f7f..db08194484e06 100644
--- a/fs/smb/client/smb2pdu.h
+++ b/fs/smb/client/smb2pdu.h
@@ -319,13 +319,15 @@ struct smb2_file_reparse_point_info {
 } __packed;
 
 struct smb2_file_network_open_info {
-	__le64 CreationTime;
-	__le64 LastAccessTime;
-	__le64 LastWriteTime;
-	__le64 ChangeTime;
-	__le64 AllocationSize;
-	__le64 EndOfFile;
-	__le32 Attributes;
+	struct_group(network_open_info,
+		__le64 CreationTime;
+		__le64 LastAccessTime;
+		__le64 LastWriteTime;
+		__le64 ChangeTime;
+		__le64 AllocationSize;
+		__le64 EndOfFile;
+		__le32 Attributes;
+	);
 	__le32 Reserved;
 } __packed; /* level 34 Query also similar returned in close rsp and open rsp */
 
diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
index 319fb9ffc6a03..7d676e7fa5b1a 100644
--- a/fs/smb/common/smb2pdu.h
+++ b/fs/smb/common/smb2pdu.h
@@ -700,13 +700,16 @@ struct smb2_close_rsp {
 	__le16 StructureSize; /* 60 */
 	__le16 Flags;
 	__le32 Reserved;
-	__le64 CreationTime;
-	__le64 LastAccessTime;
-	__le64 LastWriteTime;
-	__le64 ChangeTime;
-	__le64 AllocationSize;	/* Beginning of FILE_STANDARD_INFO equivalent */
-	__le64 EndOfFile;
-	__le32 Attributes;
+	struct_group(network_open_info,
+		__le64 CreationTime;
+		__le64 LastAccessTime;
+		__le64 LastWriteTime;
+		__le64 ChangeTime;
+		/* Beginning of FILE_STANDARD_INFO equivalent */
+		__le64 AllocationSize;
+		__le64 EndOfFile;
+		__le32 Attributes;
+	);
 } __packed;
 
 
-- 
2.42.0


  parent reply	other threads:[~2023-12-04 20:34 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-04 20:32 [PATCH AUTOSEL 6.6 01/32] pinctrl: s32cc: Avoid possible string truncation Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 02/32] kunit: Warn if tests are slow Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 03/32] kunit: Reset suite counter right before running tests Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 04/32] pinctrl: lochnagar: Don't build on MIPS Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 05/32] ALSA: hda - Fix speaker and headset mic pin config for CHUWI CoreBook XPro Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 06/32] ksmbd: separately allocate ci per dentry Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 07/32] ksmbd: release interim response after sending status pending response Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 08/32] ksmbd: move setting SMB2_FLAGS_ASYNC_COMMAND and AsyncId Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 09/32] ksmbd: don't update ->op_state as OPLOCK_STATE_NONE on error Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 10/32] mptcp: fix uninit-value in mptcp_incoming_options Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 11/32] wifi: cfg80211: lock wiphy mutex for rfkill poll Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 12/32] wifi: avoid offset calculation on NULL pointer Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 13/32] wifi: mac80211: handle 320 MHz in ieee80211_ht_cap_ie_to_sta_ht_cap Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 14/32] debugfs: fix automount d_fsdata usage Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 15/32] debugfs: annotate debugfs handlers vs. removal with lockdep Sasha Levin
2023-12-04 20:45   ` Johannes Berg
2023-12-05  4:25     ` Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 16/32] debugfs: add API to allow debugfs operations cancellation Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 17/32] wifi: cfg80211: add locked debugfs wrappers Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 18/32] wifi: mac80211: use wiphy locked debugfs helpers for agg_status Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 19/32] ALSA: hda: intel-nhlt: Ignore vbps when looking for DMIC 32 bps format Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 20/32] nvme-core: fix a memory leak in nvme_ns_info_from_identify() Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 21/32] io_uring: use fget/fput consistently Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 22/32] block: warn once for each partition in bio_check_ro() Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 23/32] drm/amdgpu: Do not issue gpu reset from nbio v7_9 bif interrupt Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 24/32] drm/amdkfd: Use common function for IP version check Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 25/32] drm/amd/display: update dcn315 lpddr pstate latency Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 26/32] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes kernel null pointer Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 27/32] drm/amdkfd: Free gang_ctx_bo and wptr_bo in pqm_uninit Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 28/32] drm/amdgpu: Use another offset for GC 9.4.3 remap Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32 ` Sasha Levin [this message]
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 30/32] Revert "drm/prime: Unexport helpers for fd/handle conversion" Sasha Levin
2023-12-04 20:32   ` Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 31/32] blk-mq: don't count completed flush data request as inflight in case of quiesce Sasha Levin
2023-12-04 20:32 ` [PATCH AUTOSEL 6.6 32/32] nvme-core: check for too small lba shift Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231204203317.2092321-29-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=dmantipov@yandex.ru \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=samba-technical@lists.samba.org \
    --cc=sfrench@samba.org \
    --cc=stable@vger.kernel.org \
    --cc=stfrench@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.