2023-12-04 20:38:04

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 01/17] pinctrl: lochnagar: Don't build on MIPS

From: Charles Keepax <[email protected]>

[ Upstream commit 6588732445ff19f6183f0fa72ddedf67e5a5be32 ]

MIPS appears to define a RST symbol at a high level, which clashes
with some register naming in the driver. Since there is currently
no case for running this driver on MIPS devices simply cut off the
build of this driver on MIPS.

Reported-by: kernel test robot <[email protected]>
Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Suggested-by: Linus Walleij <[email protected]>
Signed-off-by: Charles Keepax <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Linus Walleij <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/pinctrl/cirrus/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/cirrus/Kconfig b/drivers/pinctrl/cirrus/Kconfig
index 530426a74f751..b3cea8d56c4f6 100644
--- a/drivers/pinctrl/cirrus/Kconfig
+++ b/drivers/pinctrl/cirrus/Kconfig
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0-only
config PINCTRL_LOCHNAGAR
tristate "Cirrus Logic Lochnagar pinctrl driver"
- depends on MFD_LOCHNAGAR
+ # Avoid clash caused by MIPS defining RST, which is used in the driver
+ depends on MFD_LOCHNAGAR && !MIPS
select GPIOLIB
select PINMUX
select PINCONF
--
2.42.0


2023-12-04 20:38:08

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 04/17] mptcp: fix uninit-value in mptcp_incoming_options

From: Edward Adam Davis <[email protected]>

[ Upstream commit 237ff253f2d4f6307b7b20434d7cbcc67693298b ]

Added initialization use_ack to mptcp_parse_option().

Reported-by: [email protected]
Signed-off-by: Edward Adam Davis <[email protected]>
Acked-by: Paolo Abeni <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
net/mptcp/options.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 0c786ceda5ee6..74027bb5b4296 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -103,6 +103,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
mp_opt->suboptions |= OPTION_MPTCP_DSS;
mp_opt->use_map = 1;
mp_opt->mpc_map = 1;
+ mp_opt->use_ack = 0;
mp_opt->data_len = get_unaligned_be16(ptr);
ptr += 2;
}
--
2.42.0

2023-12-04 20:38:20

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 03/17] ksmbd: don't update ->op_state as OPLOCK_STATE_NONE on error

From: Namjae Jeon <[email protected]>

[ Upstream commit cd80ce7e68f1624ac29cd0a6b057789d1236641e ]

ksmbd set ->op_state as OPLOCK_STATE_NONE on lease break ack error.
op_state of lease should not be updated because client can send lease
break ack again. This patch fix smb2.lease.breaking2 test failure.

Signed-off-by: Namjae Jeon <[email protected]>
Signed-off-by: Steve French <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
fs/smb/server/smb2pdu.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 683152007566c..603d9170d28a7 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -8294,7 +8294,6 @@ static void smb21_lease_break_ack(struct ksmbd_work *work)
return;

err_out:
- opinfo->op_state = OPLOCK_STATE_NONE;
wake_up_interruptible_all(&opinfo->oplock_q);
atomic_dec(&opinfo->breaking_cnt);
wake_up_interruptible_all(&opinfo->oplock_brk);
--
2.42.0

2023-12-04 20:38:24

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 10/17] debugfs: add API to allow debugfs operations cancellation

From: Johannes Berg <[email protected]>

[ Upstream commit 8c88a474357ead632b07c70bf7f119ace8c3b39e ]

In some cases there might be longer-running hardware accesses
in debugfs files, or attempts to acquire locks, and we want
to still be able to quickly remove the files.

Introduce a cancellations API to use inside the debugfs handler
functions to be able to cancel such operations on a per-file
basis.

Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
fs/debugfs/file.c | 82 +++++++++++++++++++++++++++++++++++++++++
fs/debugfs/inode.c | 32 +++++++++++++++-
fs/debugfs/internal.h | 5 +++
include/linux/debugfs.h | 19 ++++++++++
4 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 375af381bf005..b3493ce50227e 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -114,6 +114,8 @@ int debugfs_file_get(struct dentry *dentry)
lockdep_init_map(&fsd->lockdep_map, fsd->lock_name ?: "debugfs",
&fsd->key, 0);
#endif
+ INIT_LIST_HEAD(&fsd->cancellations);
+ mutex_init(&fsd->cancellations_mtx);
}

/*
@@ -156,6 +158,86 @@ void debugfs_file_put(struct dentry *dentry)
}
EXPORT_SYMBOL_GPL(debugfs_file_put);

+/**
+ * debugfs_enter_cancellation - enter a debugfs cancellation
+ * @file: the file being accessed
+ * @cancellation: the cancellation object, the cancel callback
+ * inside of it must be initialized
+ *
+ * When a debugfs file is removed it needs to wait for all active
+ * operations to complete. However, the operation itself may need
+ * to wait for hardware or completion of some asynchronous process
+ * or similar. As such, it may need to be cancelled to avoid long
+ * waits or even deadlocks.
+ *
+ * This function can be used inside a debugfs handler that may
+ * need to be cancelled. As soon as this function is called, the
+ * cancellation's 'cancel' callback may be called, at which point
+ * the caller should proceed to call debugfs_leave_cancellation()
+ * and leave the debugfs handler function as soon as possible.
+ * Note that the 'cancel' callback is only ever called in the
+ * context of some kind of debugfs_remove().
+ *
+ * This function must be paired with debugfs_leave_cancellation().
+ */
+void debugfs_enter_cancellation(struct file *file,
+ struct debugfs_cancellation *cancellation)
+{
+ struct debugfs_fsdata *fsd;
+ struct dentry *dentry = F_DENTRY(file);
+
+ INIT_LIST_HEAD(&cancellation->list);
+
+ if (WARN_ON(!d_is_reg(dentry)))
+ return;
+
+ if (WARN_ON(!cancellation->cancel))
+ return;
+
+ fsd = READ_ONCE(dentry->d_fsdata);
+ if (WARN_ON(!fsd ||
+ ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)))
+ return;
+
+ mutex_lock(&fsd->cancellations_mtx);
+ list_add(&cancellation->list, &fsd->cancellations);
+ mutex_unlock(&fsd->cancellations_mtx);
+
+ /* if we're already removing wake it up to cancel */
+ if (d_unlinked(dentry))
+ complete(&fsd->active_users_drained);
+}
+EXPORT_SYMBOL_GPL(debugfs_enter_cancellation);
+
+/**
+ * debugfs_leave_cancellation - leave cancellation section
+ * @file: the file being accessed
+ * @cancellation: the cancellation previously registered with
+ * debugfs_enter_cancellation()
+ *
+ * See the documentation of debugfs_enter_cancellation().
+ */
+void debugfs_leave_cancellation(struct file *file,
+ struct debugfs_cancellation *cancellation)
+{
+ struct debugfs_fsdata *fsd;
+ struct dentry *dentry = F_DENTRY(file);
+
+ if (WARN_ON(!d_is_reg(dentry)))
+ return;
+
+ fsd = READ_ONCE(dentry->d_fsdata);
+ if (WARN_ON(!fsd ||
+ ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)))
+ return;
+
+ mutex_lock(&fsd->cancellations_mtx);
+ if (!list_empty(&cancellation->list))
+ list_del(&cancellation->list);
+ mutex_unlock(&fsd->cancellations_mtx);
+}
+EXPORT_SYMBOL_GPL(debugfs_leave_cancellation);
+
/*
* Only permit access to world-readable files when the kernel is locked down.
* We also need to exclude any file that has ways to write or alter it as root
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 8fc470aa67823..d6058e1881add 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -248,6 +248,8 @@ static void debugfs_release_dentry(struct dentry *dentry)
lockdep_unregister_key(&fsd->key);
kfree(fsd->lock_name);
#endif
+ WARN_ON(!list_empty(&fsd->cancellations));
+ mutex_destroy(&fsd->cancellations_mtx);
}

kfree(fsd);
@@ -757,8 +759,36 @@ static void __debugfs_file_removed(struct dentry *dentry)
lock_map_acquire(&fsd->lockdep_map);
lock_map_release(&fsd->lockdep_map);

- if (!refcount_dec_and_test(&fsd->active_users))
+ /* if we hit zero, just wait for all to finish */
+ if (!refcount_dec_and_test(&fsd->active_users)) {
wait_for_completion(&fsd->active_users_drained);
+ return;
+ }
+
+ /* if we didn't hit zero, try to cancel any we can */
+ while (refcount_read(&fsd->active_users)) {
+ struct debugfs_cancellation *c;
+
+ /*
+ * Lock the cancellations. Note that the cancellations
+ * structs are meant to be on the stack, so we need to
+ * ensure we either use them here or don't touch them,
+ * and debugfs_leave_cancellation() will wait for this
+ * to be finished processing before exiting one. It may
+ * of course win and remove the cancellation, but then
+ * chances are we never even got into this bit, we only
+ * do if the refcount isn't zero already.
+ */
+ mutex_lock(&fsd->cancellations_mtx);
+ while ((c = list_first_entry_or_null(&fsd->cancellations,
+ typeof(*c), list))) {
+ list_del_init(&c->list);
+ c->cancel(dentry, c->cancel_data);
+ }
+ mutex_unlock(&fsd->cancellations_mtx);
+
+ wait_for_completion(&fsd->active_users_drained);
+ }
}

static void remove_one(struct dentry *victim)
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index c7d61cfc97d26..0c4c68cf161f8 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -8,6 +8,7 @@
#ifndef _DEBUGFS_INTERNAL_H_
#define _DEBUGFS_INTERNAL_H_
#include <linux/lockdep.h>
+#include <linux/list.h>

struct file_operations;

@@ -29,6 +30,10 @@ struct debugfs_fsdata {
struct lock_class_key key;
char *lock_name;
#endif
+
+ /* protect cancellations */
+ struct mutex cancellations_mtx;
+ struct list_head cancellations;
};
};
};
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index ea2d919fd9c79..c9c65b132c0fd 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -171,6 +171,25 @@ ssize_t debugfs_write_file_bool(struct file *file, const char __user *user_buf,
ssize_t debugfs_read_file_str(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos);

+/**
+ * struct debugfs_cancellation - cancellation data
+ * @list: internal, for keeping track
+ * @cancel: callback to call
+ * @cancel_data: extra data for the callback to call
+ */
+struct debugfs_cancellation {
+ struct list_head list;
+ void (*cancel)(struct dentry *, void *);
+ void *cancel_data;
+};
+
+void __acquires(cancellation)
+debugfs_enter_cancellation(struct file *file,
+ struct debugfs_cancellation *cancellation);
+void __releases(cancellation)
+debugfs_leave_cancellation(struct file *file,
+ struct debugfs_cancellation *cancellation);
+
#else

#include <linux/err.h>
--
2.42.0

2023-12-04 20:38:25

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 11/17] ALSA: hda: intel-nhlt: Ignore vbps when looking for DMIC 32 bps format

From: Peter Ujfalusi <[email protected]>

[ Upstream commit 7b4c93a50a2ebbbaf656cc4fa6aca74a6166d85b ]

When looking up DMIC blob from the NHLT table and the format is 32 bits,
ignore the vbps matching for 32 bps for DMIC since some NHLT table have
the vbps as 24, some have it as 32.
The DMIC hardware supports only one type of 32 bit sample size, which is
24 bit sampling on the MSB side and bits[1:0] is used for indicating the
channel number.

Signed-off-by: Peter Ujfalusi <[email protected]>
Reviewed-by: Kai Vehmanen <[email protected]>
Reviewed-by: Pierre-Louis Bossart <[email protected]>
Reviewed-by: Ranjani Sridharan <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Iwai <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
sound/hda/intel-nhlt.c | 33 +++++++++++++++++++++++++++++----
1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index 2c4dfc0b7e342..696a958d93e9c 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -238,7 +238,7 @@ EXPORT_SYMBOL(intel_nhlt_ssp_mclk_mask);

static struct nhlt_specific_cfg *
nhlt_get_specific_cfg(struct device *dev, struct nhlt_fmt *fmt, u8 num_ch,
- u32 rate, u8 vbps, u8 bps)
+ u32 rate, u8 vbps, u8 bps, bool ignore_vbps)
{
struct nhlt_fmt_cfg *cfg = fmt->fmt_config;
struct wav_fmt *wfmt;
@@ -255,8 +255,12 @@ nhlt_get_specific_cfg(struct device *dev, struct nhlt_fmt *fmt, u8 num_ch,
dev_dbg(dev, "Endpoint format: ch=%d fmt=%d/%d rate=%d\n",
wfmt->channels, _vbps, _bps, wfmt->samples_per_sec);

+ /*
+ * When looking for exact match of configuration ignore the vbps
+ * from NHLT table when ignore_vbps is true
+ */
if (wfmt->channels == num_ch && wfmt->samples_per_sec == rate &&
- vbps == _vbps && bps == _bps)
+ (ignore_vbps || vbps == _vbps) && bps == _bps)
return &cfg->config;

cfg = (struct nhlt_fmt_cfg *)(cfg->config.caps + cfg->config.size);
@@ -289,6 +293,7 @@ intel_nhlt_get_endpoint_blob(struct device *dev, struct nhlt_acpi_table *nhlt,
{
struct nhlt_specific_cfg *cfg;
struct nhlt_endpoint *epnt;
+ bool ignore_vbps = false;
struct nhlt_fmt *fmt;
int i;

@@ -298,7 +303,26 @@ intel_nhlt_get_endpoint_blob(struct device *dev, struct nhlt_acpi_table *nhlt,
dev_dbg(dev, "Looking for configuration:\n");
dev_dbg(dev, " vbus_id=%d link_type=%d dir=%d, dev_type=%d\n",
bus_id, link_type, dir, dev_type);
- dev_dbg(dev, " ch=%d fmt=%d/%d rate=%d\n", num_ch, vbps, bps, rate);
+ if (link_type == NHLT_LINK_DMIC && bps == 32 && (vbps == 24 || vbps == 32)) {
+ /*
+ * The DMIC hardware supports only one type of 32 bits sample
+ * size, which is 24 bit sampling on the MSB side and bits[1:0]
+ * are used for indicating the channel number.
+ * It has been observed that some NHLT tables have the vbps
+ * specified as 32 while some uses 24.
+ * The format these variations describe are identical, the
+ * hardware is configured and behaves the same way.
+ * Note: when the samples assumed to be vbps=32 then the 'noise'
+ * introduced by the lower two bits (channel number) have no
+ * real life implication on audio quality.
+ */
+ dev_dbg(dev,
+ " ch=%d fmt=%d rate=%d (vbps is ignored for DMIC 32bit format)\n",
+ num_ch, bps, rate);
+ ignore_vbps = true;
+ } else {
+ dev_dbg(dev, " ch=%d fmt=%d/%d rate=%d\n", num_ch, vbps, bps, rate);
+ }
dev_dbg(dev, "Endpoint count=%d\n", nhlt->endpoint_count);

epnt = (struct nhlt_endpoint *)nhlt->desc;
@@ -307,7 +331,8 @@ intel_nhlt_get_endpoint_blob(struct device *dev, struct nhlt_acpi_table *nhlt,
if (nhlt_check_ep_match(dev, epnt, bus_id, link_type, dir, dev_type)) {
fmt = (struct nhlt_fmt *)(epnt->config.caps + epnt->config.size);

- cfg = nhlt_get_specific_cfg(dev, fmt, num_ch, rate, vbps, bps);
+ cfg = nhlt_get_specific_cfg(dev, fmt, num_ch, rate,
+ vbps, bps, ignore_vbps);
if (cfg)
return cfg;
}
--
2.42.0

2023-12-04 20:38:36

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 12/17] nvme-core: fix a memory leak in nvme_ns_info_from_identify()

From: Maurizio Lombardi <[email protected]>

[ Upstream commit e3139cef8257fcab1725441e2fd5fd0ccb5481b1 ]

In case of error, free the nvme_id_ns structure that was allocated
by nvme_identify_ns().

Signed-off-by: Maurizio Lombardi <[email protected]>
Reviewed-by: Sagi Grimberg <[email protected]>
Reviewed-by: Kanchan Joshi <[email protected]>
Signed-off-by: Keith Busch <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/nvme/host/core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 25ddfabc58f73..0590c0b81fca9 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1511,7 +1511,8 @@ static int nvme_ns_info_from_identify(struct nvme_ctrl *ctrl,
if (id->ncap == 0) {
/* namespace not allocated or attached */
info->is_removed = true;
- return -ENODEV;
+ ret = -ENODEV;
+ goto error;
}

info->anagrpid = id->anagrpid;
@@ -1529,8 +1530,10 @@ static int nvme_ns_info_from_identify(struct nvme_ctrl *ctrl,
!memchr_inv(ids->nguid, 0, sizeof(ids->nguid)))
memcpy(ids->nguid, id->nguid, sizeof(ids->nguid));
}
+
+error:
kfree(id);
- return 0;
+ return ret;
}

static int nvme_ns_info_from_id_cs_indep(struct nvme_ctrl *ctrl,
--
2.42.0

2023-12-04 20:38:46

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 09/17] debugfs: annotate debugfs handlers vs. removal with lockdep

From: Johannes Berg <[email protected]>

[ Upstream commit f4acfcd4deb158b96595250cc332901b282d15b0 ]

When you take a lock in a debugfs handler but also try
to remove the debugfs file under that lock, things can
deadlock since the removal has to wait for all users
to finish.

Add lockdep annotations in debugfs_file_get()/_put()
to catch such issues.

Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
fs/debugfs/file.c | 10 ++++++++++
fs/debugfs/inode.c | 12 ++++++++++++
fs/debugfs/internal.h | 6 ++++++
3 files changed, 28 insertions(+)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index b38304b444764..375af381bf005 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -108,6 +108,12 @@ int debugfs_file_get(struct dentry *dentry)
kfree(fsd);
fsd = READ_ONCE(dentry->d_fsdata);
}
+#ifdef CONFIG_LOCKDEP
+ fsd->lock_name = kasprintf(GFP_KERNEL, "debugfs:%pd", dentry);
+ lockdep_register_key(&fsd->key);
+ lockdep_init_map(&fsd->lockdep_map, fsd->lock_name ?: "debugfs",
+ &fsd->key, 0);
+#endif
}

/*
@@ -124,6 +130,8 @@ int debugfs_file_get(struct dentry *dentry)
if (!refcount_inc_not_zero(&fsd->active_users))
return -EIO;

+ lock_map_acquire_read(&fsd->lockdep_map);
+
return 0;
}
EXPORT_SYMBOL_GPL(debugfs_file_get);
@@ -141,6 +149,8 @@ void debugfs_file_put(struct dentry *dentry)
{
struct debugfs_fsdata *fsd = READ_ONCE(dentry->d_fsdata);

+ lock_map_release(&fsd->lockdep_map);
+
if (refcount_dec_and_test(&fsd->active_users))
complete(&fsd->active_users_drained);
}
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 08ef685167ec5..8fc470aa67823 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -242,6 +242,14 @@ static void debugfs_release_dentry(struct dentry *dentry)
if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
return;

+ /* check it wasn't a dir (no fsdata) or automount (no real_fops) */
+ if (fsd && fsd->real_fops) {
+#ifdef CONFIG_LOCKDEP
+ lockdep_unregister_key(&fsd->key);
+ kfree(fsd->lock_name);
+#endif
+ }
+
kfree(fsd);
}

@@ -745,6 +753,10 @@ static void __debugfs_file_removed(struct dentry *dentry)
fsd = READ_ONCE(dentry->d_fsdata);
if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
return;
+
+ lock_map_acquire(&fsd->lockdep_map);
+ lock_map_release(&fsd->lockdep_map);
+
if (!refcount_dec_and_test(&fsd->active_users))
wait_for_completion(&fsd->active_users_drained);
}
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index f7c489b5a368c..c7d61cfc97d26 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -7,6 +7,7 @@

#ifndef _DEBUGFS_INTERNAL_H_
#define _DEBUGFS_INTERNAL_H_
+#include <linux/lockdep.h>

struct file_operations;

@@ -23,6 +24,11 @@ struct debugfs_fsdata {
struct {
refcount_t active_users;
struct completion active_users_drained;
+#ifdef CONFIG_LOCKDEP
+ struct lockdep_map lockdep_map;
+ struct lock_class_key key;
+ char *lock_name;
+#endif
};
};
};
--
2.42.0

2023-12-04 20:39:09

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 08/17] debugfs: fix automount d_fsdata usage

From: Johannes Berg <[email protected]>

[ Upstream commit 0ed04a1847a10297595ac24dc7d46b35fb35f90a ]

debugfs_create_automount() stores a function pointer in d_fsdata,
but since commit 7c8d469877b1 ("debugfs: add support for more
elaborate ->d_fsdata") debugfs_release_dentry() will free it, now
conditionally on DEBUGFS_FSDATA_IS_REAL_FOPS_BIT, but that's not
set for the function pointer in automount. As a result, removing
an automount dentry would attempt to free the function pointer.
Luckily, the only user of this (tracing) never removes it.

Nevertheless, it's safer if we just handle the fsdata in one way,
namely either DEBUGFS_FSDATA_IS_REAL_FOPS_BIT or allocated. Thus,
change the automount to allocate it, and use the real_fops in the
data to indicate whether or not automount is filled, rather than
adding a type tag. At least for now this isn't actually needed,
but the next changes will require it.

Also check in debugfs_file_get() that it gets only called
on regular files, just to make things clearer.

Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
fs/debugfs/file.c | 8 ++++++++
fs/debugfs/inode.c | 27 ++++++++++++++++++++-------
fs/debugfs/internal.h | 10 ++++++++--
3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index b54f470e0d031..b38304b444764 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -84,6 +84,14 @@ int debugfs_file_get(struct dentry *dentry)
struct debugfs_fsdata *fsd;
void *d_fsd;

+ /*
+ * This could only happen if some debugfs user erroneously calls
+ * debugfs_file_get() on a dentry that isn't even a file, let
+ * them know about it.
+ */
+ if (WARN_ON(!d_is_reg(dentry)))
+ return -EINVAL;
+
d_fsd = READ_ONCE(dentry->d_fsdata);
if (!((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) {
fsd = d_fsd;
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 2e8e112b19930..08ef685167ec5 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -237,17 +237,19 @@ static const struct super_operations debugfs_super_operations = {

static void debugfs_release_dentry(struct dentry *dentry)
{
- void *fsd = dentry->d_fsdata;
+ struct debugfs_fsdata *fsd = dentry->d_fsdata;

- if (!((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))
- kfree(dentry->d_fsdata);
+ if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
+ return;
+
+ kfree(fsd);
}

static struct vfsmount *debugfs_automount(struct path *path)
{
- debugfs_automount_t f;
- f = (debugfs_automount_t)path->dentry->d_fsdata;
- return f(path->dentry, d_inode(path->dentry)->i_private);
+ struct debugfs_fsdata *fsd = path->dentry->d_fsdata;
+
+ return fsd->automount(path->dentry, d_inode(path->dentry)->i_private);
}

static const struct dentry_operations debugfs_dops = {
@@ -635,13 +637,23 @@ struct dentry *debugfs_create_automount(const char *name,
void *data)
{
struct dentry *dentry = start_creating(name, parent);
+ struct debugfs_fsdata *fsd;
struct inode *inode;

if (IS_ERR(dentry))
return dentry;

+ fsd = kzalloc(sizeof(*fsd), GFP_KERNEL);
+ if (!fsd) {
+ failed_creating(dentry);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ fsd->automount = f;
+
if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
failed_creating(dentry);
+ kfree(fsd);
return ERR_PTR(-EPERM);
}

@@ -649,13 +661,14 @@ struct dentry *debugfs_create_automount(const char *name,
if (unlikely(!inode)) {
pr_err("out of free dentries, can not create automount '%s'\n",
name);
+ kfree(fsd);
return failed_creating(dentry);
}

make_empty_dir_inode(inode);
inode->i_flags |= S_AUTOMOUNT;
inode->i_private = data;
- dentry->d_fsdata = (void *)f;
+ dentry->d_fsdata = fsd;
/* directory inodes start off with i_nlink == 2 (for "." entry) */
inc_nlink(inode);
d_instantiate(dentry, inode);
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 92af8ae313134..f7c489b5a368c 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -17,8 +17,14 @@ extern const struct file_operations debugfs_full_proxy_file_operations;

struct debugfs_fsdata {
const struct file_operations *real_fops;
- refcount_t active_users;
- struct completion active_users_drained;
+ union {
+ /* automount_fn is used when real_fops is NULL */
+ debugfs_automount_t automount;
+ struct {
+ refcount_t active_users;
+ struct completion active_users_drained;
+ };
+ };
};

/*
--
2.42.0

2023-12-04 20:39:11

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 14/17] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes kernel null pointer

From: Lu Yao <[email protected]>

[ Upstream commit 2161e09cd05a50d80736fe397145340d2e8f6c05 ]

For 'AMDGPU_FAMILY_SI' family cards, in 'si_common_early_init' func, init
'didt_rreg' and 'didt_wreg' to 'NULL'. But in func
'amdgpu_debugfs_regs_didt_read/write', using 'RREG32_DIDT' 'WREG32_DIDT'
lacks of relevant judgment. And other 'amdgpu_ip_block_version' that use
these two definitions won't be added for 'AMDGPU_FAMILY_SI'.

So, add null pointer judgment before calling.

Reviewed-by: Christian König <[email protected]>
Signed-off-by: Lu Yao <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index fd796574f87a5..8123feb1a1161 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -479,6 +479,9 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char __user *buf,
if (size & 0x3 || *pos & 0x3)
return -EINVAL;

+ if (!adev->didt_rreg)
+ return -EOPNOTSUPP;
+
r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
if (r < 0) {
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -535,6 +538,9 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file *f, const char __user
if (size & 0x3 || *pos & 0x3)
return -EINVAL;

+ if (!adev->didt_wreg)
+ return -EOPNOTSUPP;
+
r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
if (r < 0) {
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
--
2.42.0

2023-12-04 20:39:20

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 13/17] drm/amd/display: update dcn315 lpddr pstate latency

From: Dmytro Laktyushkin <[email protected]>

[ Upstream commit c92da0403d373c03ea5c65c0260c7db6762013b0 ]

[WHY/HOW]
Increase the pstate latency to improve ac/dc transition

Reviewed-by: Charlene Liu <[email protected]>
Acked-by: Tom Chung <[email protected]>
Signed-off-by: Dmytro Laktyushkin <[email protected]>
Tested-by: Daniel Wheeler <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
.../drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
index 893991a0eb971..28b83133db910 100644
--- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
+++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn315/dcn315_clk_mgr.c
@@ -324,7 +324,7 @@ static struct wm_table lpddr5_wm_table = {
{
.wm_inst = WM_A,
.wm_type = WM_TYPE_PSTATE_CHG,
- .pstate_latency_us = 11.65333,
+ .pstate_latency_us = 129.0,
.sr_exit_time_us = 11.5,
.sr_enter_plus_exit_time_us = 14.5,
.valid = true,
@@ -332,7 +332,7 @@ static struct wm_table lpddr5_wm_table = {
{
.wm_inst = WM_B,
.wm_type = WM_TYPE_PSTATE_CHG,
- .pstate_latency_us = 11.65333,
+ .pstate_latency_us = 129.0,
.sr_exit_time_us = 11.5,
.sr_enter_plus_exit_time_us = 14.5,
.valid = true,
@@ -340,7 +340,7 @@ static struct wm_table lpddr5_wm_table = {
{
.wm_inst = WM_C,
.wm_type = WM_TYPE_PSTATE_CHG,
- .pstate_latency_us = 11.65333,
+ .pstate_latency_us = 129.0,
.sr_exit_time_us = 11.5,
.sr_enter_plus_exit_time_us = 14.5,
.valid = true,
@@ -348,7 +348,7 @@ static struct wm_table lpddr5_wm_table = {
{
.wm_inst = WM_D,
.wm_type = WM_TYPE_PSTATE_CHG,
- .pstate_latency_us = 11.65333,
+ .pstate_latency_us = 129.0,
.sr_exit_time_us = 11.5,
.sr_enter_plus_exit_time_us = 14.5,
.valid = true,
--
2.42.0

2023-12-04 20:39:28

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 15/17] smb: client, common: fix fortify warnings

From: Dmitry Antipov <[email protected]>

[ 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 <[email protected]>
Acked-by: Namjae Jeon <[email protected]>
Signed-off-by: Steve French <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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 c403816d0b6c1..97bb1838555b4 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;
@@ -2268,11 +2270,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 c90d4ec9292ca..67c5fc2b2db94 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1234,8 +1234,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 847d69d327c2a..aea7770fb5631 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3425,12 +3425,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 1237bb86e93a8..8ac99563487c1 100644
--- a/fs/smb/client/smb2pdu.h
+++ b/fs/smb/client/smb2pdu.h
@@ -339,13 +339,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 9619015d78f29..778c1e3b70bc1 100644
--- a/fs/smb/common/smb2pdu.h
+++ b/fs/smb/common/smb2pdu.h
@@ -699,13 +699,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

2023-12-04 20:39:34

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 16/17] blk-mq: don't count completed flush data request as inflight in case of quiesce

From: Ming Lei <[email protected]>

[ Upstream commit 0e4237ae8d159e3d28f3cd83146a46f576ffb586 ]

Request queue quiesce may interrupt flush sequence, and the original request
may have been marked as COMPLETE, but can't get finished because of
queue quiesce.

This way is fine from driver viewpoint, because flush sequence is block
layer concept, and it isn't related with driver.

However, driver(such as dm-rq) can call blk_mq_queue_inflight() to count &
drain inflight requests, then the wait & drain never gets done because
the completed & not-finished flush request is counted as inflight.

Fix this issue by not counting completed flush data request as inflight in
case of quiesce.

Cc: Mike Snitzer <[email protected]>
Cc: David Jeffery <[email protected]>
Cc: John Pittman <[email protected]>
Signed-off-by: Ming Lei <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jens Axboe <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
block/blk-mq.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 100fb0c3114f8..70fc5fd27a5d9 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1500,14 +1500,26 @@ void blk_mq_delay_kick_requeue_list(struct request_queue *q,
}
EXPORT_SYMBOL(blk_mq_delay_kick_requeue_list);

+static bool blk_is_flush_data_rq(struct request *rq)
+{
+ return (rq->rq_flags & RQF_FLUSH_SEQ) && !is_flush_rq(rq);
+}
+
static bool blk_mq_rq_inflight(struct request *rq, void *priv)
{
/*
* If we find a request that isn't idle we know the queue is busy
* as it's checked in the iter.
* Return false to stop the iteration.
+ *
+ * In case of queue quiesce, if one flush data request is completed,
+ * don't count it as inflight given the flush sequence is suspended,
+ * and the original flush data request is invisible to driver, just
+ * like other pending requests because of quiesce
*/
- if (blk_mq_request_started(rq)) {
+ if (blk_mq_request_started(rq) && !(blk_queue_quiesced(rq->q) &&
+ blk_is_flush_data_rq(rq) &&
+ blk_mq_request_completed(rq))) {
bool *busy = priv;

*busy = true;
--
2.42.0

2023-12-04 20:41:12

by Sasha Levin

[permalink] [raw]
Subject: [PATCH AUTOSEL 6.1 17/17] nvme-core: check for too small lba shift

From: Keith Busch <[email protected]>

[ Upstream commit 74fbc88e161424b3b96a22b23a8e3e1edab9d05c ]

The block layer doesn't support logical block sizes smaller than 512
bytes. The nvme spec doesn't support that small either, but the driver
isn't checking to make sure the device responded with usable data.
Failing to catch this will result in a kernel bug, either from a
division by zero when stacking, or a zero length bio.

Reviewed-by: Jens Axboe <[email protected]>
Signed-off-by: Keith Busch <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/nvme/host/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 0590c0b81fca9..b0db3b54d69a3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1922,9 +1922,10 @@ static void nvme_update_disk_info(struct gendisk *disk,

/*
* The block layer can't support LBA sizes larger than the page size
- * yet, so catch this early and don't allow block I/O.
+ * or smaller than a sector size yet, so catch this early and don't
+ * allow block I/O.
*/
- if (ns->lba_shift > PAGE_SHIFT) {
+ if (ns->lba_shift > PAGE_SHIFT || ns->lba_shift < SECTOR_SHIFT) {
capacity = 0;
bs = (1 << 9);
}
--
2.42.0

2024-04-23 18:58:13

by Steve French

[permalink] [raw]
Subject: Re: [PATCH AUTOSEL 6.1 15/17] smb: client, common: fix fortify warnings

Note that kernels that backported this fix will also need this ksmbd
fix (fixes a bug when Macs mount to ksmbd)

commit 0268a7cc7fdc47d90b6c18859de7718d5059f6f1
Author: Namjae Jeon <[email protected]>
Date: Fri Apr 19 23:46:34 2024 +0900

ksmbd: common: use struct_group_attr instead of struct_group for
network_open_info

4byte padding cause the connection issue with the applications of MacOS.
smb2_close response size increases by 4 bytes by padding, And the smb
client of MacOS check it and stop the connection. This patch use
struct_group_attr instead of struct_group for network_open_info to use
__packed to avoid padding.

Fixes: 0015eb6e1238 ("smb: client, common: fix fortify warnings")
Cc: [email protected]
Signed-off-by: Namjae Jeon <[email protected]>
Signed-off-by: Steve French <[email protected]>

On Mon, Dec 4, 2023 at 2:36 PM Sasha Levin <[email protected]> wrote:
>
> From: Dmitry Antipov <[email protected]>
>
> [ 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 <[email protected]>
> Acked-by: Namjae Jeon <[email protected]>
> Signed-off-by: Steve French <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> 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 c403816d0b6c1..97bb1838555b4 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;
> @@ -2268,11 +2270,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 c90d4ec9292ca..67c5fc2b2db94 100644
> --- a/fs/smb/client/cifssmb.c
> +++ b/fs/smb/client/cifssmb.c
> @@ -1234,8 +1234,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 847d69d327c2a..aea7770fb5631 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -3425,12 +3425,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 1237bb86e93a8..8ac99563487c1 100644
> --- a/fs/smb/client/smb2pdu.h
> +++ b/fs/smb/client/smb2pdu.h
> @@ -339,13 +339,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 9619015d78f29..778c1e3b70bc1 100644
> --- a/fs/smb/common/smb2pdu.h
> +++ b/fs/smb/common/smb2pdu.h
> @@ -699,13 +699,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
>
>


--
Thanks,

Steve