May 2024 - Linux-stable-mirror

[PATCH v3] can: mcp251xfd: fix infinite loop when xmit fails

by Vitor Soares

From: Vitor Soares <vitor.soares(a)toradex.com> When the mcp251xfd_start_xmit() function fails, the driver stops processing messages, and the interrupt routine does not return, running indefinitely even after killing the running application. Error messages: [ 441.298819] mcp251xfd spi2.0 can0: ERROR in mcp251xfd_start_xmit: -16 [ 441.306498] mcp251xfd spi2.0 can0: Transmit Event FIFO buffer not empty. (seq=0x000017c7, tef_tail=0x000017cf, tef_head=0x000017d0, tx_head=0x000017d3). ... and repeat forever. The issue can be triggered when multiple devices share the same SPI interface. And there is concurrent access to the bus. The problem occurs because tx_ring->head increments even if mcp251xfd_start_xmit() fails. Consequently, the driver skips one TX package while still expecting a response in mcp251xfd_handle_tefif_one(). This patch resolves the issue by decreasing tx_ring->head if mcp251xfd_start_xmit() fails. With the fix, if we trigger the issue and the err = -EBUSY, the driver returns NETDEV_TX_BUSY. The network stack retries to transmit the message. Otherwise, it prints an error and discards the message. Fixes: 55e5b97f003e ("can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN") Cc: stable(a)vger.kernel.org Signed-off-by: Vitor Soares <vitor.soares(a)toradex.com> --- V2->V3: - Add tx_dropped stats. - netdev_sent_queue() only if can_put_echo_skb() succeed. V1->V2: - Return NETDEV_TX_BUSY if mcp251xfd_tx_obj_write() == -EBUSY. - Rework the commit message to address the change above. - Change can_put_echo_skb() to be called after mcp251xfd_tx_obj_write() succeed. Otherwise, we get Kernel NULL pointer dereference error. drivers/net/can/spi/mcp251xfd/mcp251xfd-tx.c | 34 ++++++++++++-------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-tx.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-tx.c index 160528d3cc26..146c44e47c60 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-tx.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-tx.c @@ -166,6 +166,7 @@ netdev_tx_t mcp251xfd_start_xmit(struct sk_buff *skb, struct net_device *ndev) { struct mcp251xfd_priv *priv = netdev_priv(ndev); + struct net_device_stats *stats = &ndev->stats; struct mcp251xfd_tx_ring *tx_ring = priv->tx; struct mcp251xfd_tx_obj *tx_obj; unsigned int frame_len; @@ -181,25 +182,32 @@ netdev_tx_t mcp251xfd_start_xmit(struct sk_buff *skb, tx_obj = mcp251xfd_get_tx_obj_next(tx_ring); mcp251xfd_tx_obj_from_skb(priv, tx_obj, skb, tx_ring->head); - /* Stop queue if we occupy the complete TX FIFO */ tx_head = mcp251xfd_get_tx_head(tx_ring); - tx_ring->head++; - if (mcp251xfd_get_tx_free(tx_ring) == 0) - netif_stop_queue(ndev); - frame_len = can_skb_get_frame_len(skb); - err = can_put_echo_skb(skb, ndev, tx_head, frame_len); - if (!err) - netdev_sent_queue(priv->ndev, frame_len); + + tx_ring->head++; err = mcp251xfd_tx_obj_write(priv, tx_obj); - if (err) - goto out_err; + if (err) { + tx_ring->head--; - return NETDEV_TX_OK; + if (err == -EBUSY) + return NETDEV_TX_BUSY; - out_err: - netdev_err(priv->ndev, "ERROR in %s: %d\n", __func__, err); + stats->tx_dropped++; + + if (net_ratelimit()) + netdev_err(priv->ndev, + "ERROR in %s: %d\n", __func__, err); + } else { + err = can_put_echo_skb(skb, ndev, tx_head, frame_len); + if (!err) + netdev_sent_queue(priv->ndev, frame_len); + + /* Stop queue if we occupy the complete TX FIFO */ + if (mcp251xfd_get_tx_free(tx_ring) == 0) + netif_stop_queue(ndev); + } return NETDEV_TX_OK; } -- 2.34.1

12 months

4
4
0 0

[PATCH 1/1] x86/pci: Skip early E820 check for ECAM region

by Bjorn Helgaas

From: Bjorn Helgaas <bhelgaas(a)google.com> Arul, Mateusz, Imcarneiro91, and Aman reported a regression caused by 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map"). On the Lenovo Legion 9i laptop, that commit removes the area containing ECAM from E820, which means the early E820 validation started failing, which meant we didn't enable ECAM in the "early MCFG" path The lack of ECAM caused many ACPI methods to fail, resulting in the embedded controller, PS/2, audio, trackpad, and battery devices not being detected. The _OSC method also failed, so Linux could not take control of the PCIe hotplug, PME, and AER features: # pci_mmcfg_early_init() PCI: ECAM [mem 0xc0000000-0xce0fffff] (base 0xc0000000) for domain 0000 [bus 00-e0] PCI: not using ECAM ([mem 0xc0000000-0xce0fffff] not reserved) ACPI Error: AE_ERROR, Returned by Handler for [PCI_Config] (20230628/evregion-300) ACPI: Interpreter enabled ACPI: Ignoring error and continuing table load ACPI BIOS Error (bug): Could not resolve symbol [\_SB.PC00.RP01._SB.PC00], AE_NOT_FOUND (20230628/dswload2-162) ACPI Error: AE_NOT_FOUND, During name lookup/catalog (20230628/psobject-220) ACPI: Skipping parse of AML opcode: OpcodeName unavailable (0x0010) ACPI BIOS Error (bug): Could not resolve symbol [\_SB.PC00.RP01._SB.PC00], AE_NOT_FOUND (20230628/dswload2-162) ACPI Error: AE_NOT_FOUND, During name lookup/catalog (20230628/psobject-220) ... ACPI Error: Aborting method \_SB.PC00._OSC due to previous error (AE_NOT_FOUND) (20230628/psparse-529) acpi PNP0A08:00: _OSC: platform retains control of PCIe features (AE_NOT_FOUND) # pci_mmcfg_late_init() PCI: ECAM [mem 0xc0000000-0xce0fffff] (base 0xc0000000) for domain 0000 [bus 00-e0] PCI: [Firmware Info]: ECAM [mem 0xc0000000-0xce0fffff] not reserved in ACPI motherboard resources PCI: ECAM [mem 0xc0000000-0xce0fffff] is EfiMemoryMappedIO; assuming valid PCI: ECAM [mem 0xc0000000-0xce0fffff] reserved to work around lack of ACPI motherboard _CRS Per PCI Firmware r3.3, sec 4.1.2, ECAM space must be reserved by a PNP0C02 resource, but it need not be mentioned in E820, so we shouldn't look at E820 to validate the ECAM space described by MCFG. 946f2ee5c731 ("[PATCH] i386/x86-64: Check that MCFG points to an e820 reserved area") added a sanity check of E820 to work around buggy MCFG tables, but that over-aggressive validation causes failures like this one. Keep the E820 validation check only for older BIOSes (pre-2016) so the buggy 2006-era machines don't break. Skip the early E820 check for 2016 and newer BIOSes. Fixes: 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map") Reported-by: Mateusz Kaduk <mateusz.kaduk(a)gmail.com> Reported-by: Arul <...> Reported-by: Imcarneiro91 <...> Reported-by: Aman <...> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218444 Signed-off-by: Bjorn Helgaas <bhelgaas(a)google.com> Tested-by: Mateusz Kaduk <mateusz.kaduk(a)gmail.com> Cc: stable(a)vger.kernel.org --- arch/x86/pci/mmconfig-shared.c | 35 +++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c index 0cc9520666ef..53c7afa606c3 100644 --- a/arch/x86/pci/mmconfig-shared.c +++ b/arch/x86/pci/mmconfig-shared.c @@ -518,7 +518,34 @@ static bool __ref pci_mmcfg_reserved(struct device *dev, { struct resource *conflict; - if (!early && !acpi_disabled) { + if (early) { + + /* + * Don't try to do this check unless configuration type 1 + * is available. How about type 2? + */ + + /* + * 946f2ee5c731 ("Check that MCFG points to an e820 + * reserved area") added this E820 check in 2006 to work + * around BIOS defects. + * + * Per PCI Firmware r3.3, sec 4.1.2, ECAM space must be + * reserved by a PNP0C02 resource, but it need not be + * mentioned in E820. Before the ACPI interpreter is + * available, we can't check for PNP0C02 resources, so + * there's no reliable way to verify the region in this + * early check. Keep it only for the old machines that + * motivated 946f2ee5c731. + */ + if (dmi_get_bios_year() < 2016 && raw_pci_ops) + return is_mmconf_reserved(e820__mapped_all, cfg, dev, + "E820 entry"); + + return true; + } + + if (!acpi_disabled) { if (is_mmconf_reserved(is_acpi_reserved, cfg, dev, "ACPI motherboard resource")) return true; @@ -554,12 +581,6 @@ static bool __ref pci_mmcfg_reserved(struct device *dev, if (pci_mmcfg_running_state) return true; - /* Don't try to do this check unless configuration - type 1 is available. how about type 2 ?*/ - if (raw_pci_ops) - return is_mmconf_reserved(e820__mapped_all, cfg, dev, - "E820 entry"); - return false; } -- 2.34.1

12 months

4
8
0 0

[PATCH v3 6/6] eventfs: Have "events" directory get permissions from its parent

by Steven Rostedt

From: "Steven Rostedt (Google)" <rostedt(a)goodmis.org> The events directory gets its permissions from the root inode. But this can cause an inconsistency if the instances directory changes its permissions, as the permissions of the created directories under it should inherit the permissions of the instances directory when directories under it are created. Currently the behavior is: # cd /sys/kernel/tracing # chgrp 1002 instances # mkdir instances/foo # ls -l instances/foo [..] -r--r----- 1 root lkp 0 May 1 18:55 buffer_total_size_kb -rw-r----- 1 root lkp 0 May 1 18:55 current_tracer -rw-r----- 1 root lkp 0 May 1 18:55 error_log drwxr-xr-x 1 root root 0 May 1 18:55 events --w------- 1 root lkp 0 May 1 18:55 free_buffer drwxr-x--- 2 root lkp 0 May 1 18:55 options drwxr-x--- 10 root lkp 0 May 1 18:55 per_cpu -rw-r----- 1 root lkp 0 May 1 18:55 set_event All the files and directories under "foo" has the "lkp" group except the "events" directory. That's because its getting its default value from the mount point instead of its parent. Have the "events" directory make its default value based on its parent's permissions. That now gives: # ls -l instances/foo [..] -rw-r----- 1 root lkp 0 May 1 21:16 buffer_subbuf_size_kb -r--r----- 1 root lkp 0 May 1 21:16 buffer_total_size_kb -rw-r----- 1 root lkp 0 May 1 21:16 current_tracer -rw-r----- 1 root lkp 0 May 1 21:16 error_log drwxr-xr-x 1 root lkp 0 May 1 21:16 events --w------- 1 root lkp 0 May 1 21:16 free_buffer drwxr-x--- 2 root lkp 0 May 1 21:16 options drwxr-x--- 10 root lkp 0 May 1 21:16 per_cpu -rw-r----- 1 root lkp 0 May 1 21:16 set_event Cc: stable(a)vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) <rostedt(a)goodmis.org> --- fs/tracefs/event_inode.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 6e08405892ae..a878cea70f4c 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -37,6 +37,7 @@ static DEFINE_MUTEX(eventfs_mutex); struct eventfs_root_inode { struct eventfs_inode ei; + struct inode *parent_inode; struct dentry *events_dir; }; @@ -226,12 +227,23 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, static void update_events_attr(struct eventfs_inode *ei, struct super_block *sb) { - struct inode *root; + struct eventfs_root_inode *rei; + struct inode *parent; + + rei = get_root_inode(ei); + + /* Use the parent inode permissions unless root set its permissions */ + parent = rei->parent_inode; - /* Get the tracefs root inode. */ - root = d_inode(sb->s_root); - ei->attr.uid = root->i_uid; - ei->attr.gid = root->i_gid; + if (rei->ei.attr.mode & EVENTFS_SAVE_UID) + ei->attr.uid = rei->ei.attr.uid; + else + ei->attr.uid = parent->i_uid; + + if (rei->ei.attr.mode & EVENTFS_SAVE_GID) + ei->attr.gid = rei->ei.attr.gid; + else + ei->attr.gid = parent->i_gid; } static void set_top_events_ownership(struct inode *inode) @@ -817,6 +829,7 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry // Note: we have a ref to the dentry from tracefs_start_creating() rei = get_root_inode(ei); rei->events_dir = dentry; + rei->parent_inode = d_inode(dentry->d_sb->s_root); ei->entries = entries; ei->nr_entries = size; @@ -826,10 +839,15 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry uid = d_inode(dentry->d_parent)->i_uid; gid = d_inode(dentry->d_parent)->i_gid; - /* This is used as the default ownership of the files and directories */ ei->attr.uid = uid; ei->attr.gid = gid; + /* + * When the "events" directory is created, it takes on the + * permissions of its parent. But can be reset on remount. + */ + ei->attr.mode |= EVENTFS_SAVE_UID | EVENTFS_SAVE_GID; + INIT_LIST_HEAD(&ei->children); INIT_LIST_HEAD(&ei->list); -- 2.43.0

12 months

1
0
0 0

[PATCH v3 5/6] eventfs: Do not treat events directory different than other directories

by Steven Rostedt

From: "Steven Rostedt (Google)" <rostedt(a)goodmis.org> Treat the events directory the same as other directories when it comes to permissions. The events directory was considered different because it's dentry is persistent, whereas the other directory dentries are created when accessed. But the way tracefs now does its ownership by using the root dentry's permissions as the default permissions, the events directory can get out of sync when a remount is performed setting the group and user permissions. Remove the special case for the events directory on setting the attributes. This allows the updates caused by remount to work properly as well as simplifies the code. Cc: stable(a)vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) <rostedt(a)goodmis.org> --- fs/tracefs/event_inode.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 9dacf65c0b6e..6e08405892ae 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -206,21 +206,7 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, * determined by the parent directory. */ if (dentry->d_inode->i_mode & S_IFDIR) { - /* - * The events directory dentry is never freed, unless its - * part of an instance that is deleted. It's attr is the - * default for its child files and directories. - * Do not update it. It's not used for its own mode or ownership. - */ - if (ei->is_events) { - /* But it still needs to know if it was modified */ - if (iattr->ia_valid & ATTR_UID) - ei->attr.mode |= EVENTFS_SAVE_UID; - if (iattr->ia_valid & ATTR_GID) - ei->attr.mode |= EVENTFS_SAVE_GID; - } else { - update_attr(&ei->attr, iattr); - } + update_attr(&ei->attr, iattr); } else { name = dentry->d_name.name; -- 2.43.0

12 months

1
0
0 0

[PATCH v3 4/6] eventfs: Do not differentiate the toplevel events directory

by Steven Rostedt

From: "Steven Rostedt (Google)" <rostedt(a)goodmis.org> The toplevel events directory is really no different than the events directory of instances. Having the two be different caused inconsistencies and made it harder to fix the permissions bugs. Make all events directories act the same. Cc: stable(a)vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) <rostedt(a)goodmis.org> --- fs/tracefs/event_inode.c | 29 ++++++++--------------------- fs/tracefs/internal.h | 7 +++---- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 15a2a9c3c62b..9dacf65c0b6e 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -68,7 +68,6 @@ enum { EVENTFS_SAVE_MODE = BIT(16), EVENTFS_SAVE_UID = BIT(17), EVENTFS_SAVE_GID = BIT(18), - EVENTFS_TOPLEVEL = BIT(19), }; #define EVENTFS_MODE_MASK (EVENTFS_SAVE_MODE - 1) @@ -239,14 +238,10 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry, return ret; } -static void update_top_events_attr(struct eventfs_inode *ei, struct super_block *sb) +static void update_events_attr(struct eventfs_inode *ei, struct super_block *sb) { struct inode *root; - /* Only update if the "events" was on the top level */ - if (!ei || !(ei->attr.mode & EVENTFS_TOPLEVEL)) - return; - /* Get the tracefs root inode. */ root = d_inode(sb->s_root); ei->attr.uid = root->i_uid; @@ -259,10 +254,10 @@ static void set_top_events_ownership(struct inode *inode) struct eventfs_inode *ei = ti->private; /* The top events directory doesn't get automatically updated */ - if (!ei || !ei->is_events || !(ei->attr.mode & EVENTFS_TOPLEVEL)) + if (!ei || !ei->is_events) return; - update_top_events_attr(ei, inode->i_sb); + update_events_attr(ei, inode->i_sb); if (!(ei->attr.mode & EVENTFS_SAVE_UID)) inode->i_uid = ei->attr.uid; @@ -291,7 +286,7 @@ static int eventfs_permission(struct mnt_idmap *idmap, return generic_permission(idmap, inode, mask); } -static const struct inode_operations eventfs_root_dir_inode_operations = { +static const struct inode_operations eventfs_dir_inode_operations = { .lookup = eventfs_root_lookup, .setattr = eventfs_set_attr, .getattr = eventfs_get_attr, @@ -359,7 +354,7 @@ static struct eventfs_inode *eventfs_find_events(struct dentry *dentry) // Walk upwards until you find the events inode } while (!ei->is_events); - update_top_events_attr(ei, dentry->d_sb); + update_events_attr(ei, dentry->d_sb); return ei; } @@ -465,7 +460,7 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry, update_inode_attr(dentry, inode, &ei->attr, S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO); - inode->i_op = &eventfs_root_dir_inode_operations; + inode->i_op = &eventfs_dir_inode_operations; inode->i_fop = &eventfs_file_operations; /* All directories will have the same inode number */ @@ -845,14 +840,6 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry uid = d_inode(dentry->d_parent)->i_uid; gid = d_inode(dentry->d_parent)->i_gid; - /* - * If the events directory is of the top instance, then parent - * is NULL. Set the attr.mode to reflect this and its permissions will - * default to the tracefs root dentry. - */ - if (!parent) - ei->attr.mode = EVENTFS_TOPLEVEL; - /* This is used as the default ownership of the files and directories */ ei->attr.uid = uid; ei->attr.gid = gid; @@ -861,13 +848,13 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry INIT_LIST_HEAD(&ei->list); ti = get_tracefs(inode); - ti->flags |= TRACEFS_EVENT_INODE | TRACEFS_EVENT_TOP_INODE; + ti->flags |= TRACEFS_EVENT_INODE; ti->private = ei; inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO; inode->i_uid = uid; inode->i_gid = gid; - inode->i_op = &eventfs_root_dir_inode_operations; + inode->i_op = &eventfs_dir_inode_operations; inode->i_fop = &eventfs_file_operations; dentry->d_fsdata = get_ei(ei); diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h index 29f0c999975b..f704d8348357 100644 --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -4,10 +4,9 @@ enum { TRACEFS_EVENT_INODE = BIT(1), - TRACEFS_EVENT_TOP_INODE = BIT(2), - TRACEFS_GID_PERM_SET = BIT(3), - TRACEFS_UID_PERM_SET = BIT(4), - TRACEFS_INSTANCE_INODE = BIT(5), + TRACEFS_GID_PERM_SET = BIT(2), + TRACEFS_UID_PERM_SET = BIT(3), + TRACEFS_INSTANCE_INODE = BIT(4), }; struct tracefs_inode { -- 2.43.0

12 months

1
0
0 0

[PATCH v3 3/6] tracefs: Still use mount point as default permissions for instances

by Steven Rostedt

From: "Steven Rostedt (Google)" <rostedt(a)goodmis.org> If the instances directory's permissions were never change, then have it and its children use the mount point permissions as the default. Currently, the permissions of instance directories are determined by the instance directory's permissions itself. But if the tracefs file system is remounted and changes the permissions, the instance directory and its children should use the new permission. But because both the instance directory and its children use the instance directory's inode for permissions, it misses the update. To demonstrate this: # cd /sys/kernel/tracing/ # mkdir instances/foo # ls -ld instances/foo drwxr-x--- 5 root root 0 May 1 19:07 instances/foo # ls -ld instances drwxr-x--- 3 root root 0 May 1 18:57 instances # ls -ld current_tracer -rw-r----- 1 root root 0 May 1 18:57 current_tracer # mount -o remount,gid=1002 . # ls -ld instances drwxr-x--- 3 root root 0 May 1 18:57 instances # ls -ld instances/foo/ drwxr-x--- 5 root root 0 May 1 19:07 instances/foo/ # ls -ld current_tracer -rw-r----- 1 root lkp 0 May 1 18:57 current_tracer Notice that changing the group id to that of "lkp" did not affect the instances directory nor its children. It should have been: # ls -ld current_tracer -rw-r----- 1 root root 0 May 1 19:19 current_tracer # ls -ld instances/foo/ drwxr-x--- 5 root root 0 May 1 19:25 instances/foo/ # ls -ld instances drwxr-x--- 3 root root 0 May 1 19:19 instances # mount -o remount,gid=1002 . # ls -ld current_tracer -rw-r----- 1 root lkp 0 May 1 19:19 current_tracer # ls -ld instances drwxr-x--- 3 root lkp 0 May 1 19:19 instances # ls -ld instances/foo/ drwxr-x--- 5 root lkp 0 May 1 19:25 instances/foo/ Where all files were updated by the remount gid update. Cc: stable(a)vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) <rostedt(a)goodmis.org> --- fs/tracefs/inode.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c index 52aa14bd2994..417c840e6403 100644 --- a/fs/tracefs/inode.c +++ b/fs/tracefs/inode.c @@ -180,16 +180,39 @@ static void set_tracefs_inode_owner(struct inode *inode) { struct tracefs_inode *ti = get_tracefs(inode); struct inode *root_inode = ti->private; + kuid_t uid; + kgid_t gid; + + uid = root_inode->i_uid; + gid = root_inode->i_gid; + + /* + * If the root is not the mount point, then check the root's + * permissions. If it was never set, then default to the + * mount point. + */ + if (root_inode != d_inode(root_inode->i_sb->s_root)) { + struct tracefs_inode *rti; + + rti = get_tracefs(root_inode); + root_inode = d_inode(root_inode->i_sb->s_root); + + if (!(rti->flags & TRACEFS_UID_PERM_SET)) + uid = root_inode->i_uid; + + if (!(rti->flags & TRACEFS_GID_PERM_SET)) + gid = root_inode->i_gid; + } /* * If this inode has never been referenced, then update * the permissions to the superblock. */ if (!(ti->flags & TRACEFS_UID_PERM_SET)) - inode->i_uid = root_inode->i_uid; + inode->i_uid = uid; if (!(ti->flags & TRACEFS_GID_PERM_SET)) - inode->i_gid = root_inode->i_gid; + inode->i_gid = gid; } static int tracefs_permission(struct mnt_idmap *idmap, -- 2.43.0

12 months

1
0
0 0

[PATCH v3 2/6] tracefs: Reset permissions on remount if permissions are options

by Steven Rostedt

From: "Steven Rostedt (Google)" <rostedt(a)goodmis.org> There's an inconsistency with the way permissions are handled in tracefs. Because the permissions are generated when accessed, they default to the root inode's permission if they were never set by the user. If the user sets the permissions, then a flag is set and the permissions are saved via the inode (for tracefs files) or an internal attribute field (for eventfs). But if a remount happens that specify the permissions, all the files that were not changed by the user gets updated, but the ones that were are not. If the user were to remount the file system with a given permission, then all files and directories within that file system should be updated. This can cause security issues if a file's permission was updated but the admin forgot about it. They could incorrectly think that remounting with permissions set would update all files, but miss some. For example: # cd /sys/kernel/tracing # chgrp 1002 current_tracer # ls -l [..] -rw-r----- 1 root root 0 May 1 21:25 buffer_size_kb -rw-r----- 1 root root 0 May 1 21:25 buffer_subbuf_size_kb -r--r----- 1 root root 0 May 1 21:25 buffer_total_size_kb -rw-r----- 1 root lkp 0 May 1 21:25 current_tracer -rw-r----- 1 root root 0 May 1 21:25 dynamic_events -r--r----- 1 root root 0 May 1 21:25 dyn_ftrace_total_info -r--r----- 1 root root 0 May 1 21:25 enabled_functions Where current_tracer now has group "lkp". # mount -o remount,gid=1001 . # ls -l -rw-r----- 1 root tracing 0 May 1 21:25 buffer_size_kb -rw-r----- 1 root tracing 0 May 1 21:25 buffer_subbuf_size_kb -r--r----- 1 root tracing 0 May 1 21:25 buffer_total_size_kb -rw-r----- 1 root lkp 0 May 1 21:25 current_tracer -rw-r----- 1 root tracing 0 May 1 21:25 dynamic_events -r--r----- 1 root tracing 0 May 1 21:25 dyn_ftrace_total_info -r--r----- 1 root tracing 0 May 1 21:25 enabled_functions Everything changed but the "current_tracer". Add a new link list that keeps track of all the tracefs_inodes which has the permission flags that tell if the file/dir should use the root inode's permission or not. Then on remount, clear all the flags so that the default behavior of using the root inode's permission is done for all files and directories. Cc: stable(a)vger.kernel.org Fixes: 8186fff7ab649 ("tracefs/eventfs: Use root and instance inodes as default ownership") Signed-off-by: Steven Rostedt (Google) <rostedt(a)goodmis.org> --- fs/tracefs/event_inode.c | 29 ++++++++++++++++++ fs/tracefs/inode.c | 65 +++++++++++++++++++++++++++++++++++++++- fs/tracefs/internal.h | 7 ++++- 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index cc8b838bbe62..15a2a9c3c62b 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -308,6 +308,35 @@ static const struct file_operations eventfs_file_operations = { .llseek = generic_file_llseek, }; +/* + * On a remount of tracefs, if UID or GID options are set, then + * the mount point inode permissions should be used. + * Reset the saved permission flags appropriately. + */ +void eventfs_remount(struct tracefs_inode *ti, bool update_uid, bool update_gid) +{ + struct eventfs_inode *ei = ti->private; + + if (!ei) + return; + + if (update_uid) + ei->attr.mode &= ~EVENTFS_SAVE_UID; + + if (update_gid) + ei->attr.mode &= ~EVENTFS_SAVE_GID; + + if (!ei->entry_attrs) + return; + + for (int i = 0; i < ei->nr_entries; i++) { + if (update_uid) + ei->entry_attrs[i].mode &= ~EVENTFS_SAVE_UID; + if (update_gid) + ei->entry_attrs[i].mode &= ~EVENTFS_SAVE_GID; + } +} + /* Return the evenfs_inode of the "events" directory */ static struct eventfs_inode *eventfs_find_events(struct dentry *dentry) { diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c index 5545e6bf7d26..52aa14bd2994 100644 --- a/fs/tracefs/inode.c +++ b/fs/tracefs/inode.c @@ -30,20 +30,47 @@ static struct vfsmount *tracefs_mount; static int tracefs_mount_count; static bool tracefs_registered; +/* + * Keep track of all tracefs_inodes in order to update their + * flags if necessary on a remount. + */ +static DEFINE_SPINLOCK(tracefs_inode_lock); +static LIST_HEAD(tracefs_inodes); + static struct inode *tracefs_alloc_inode(struct super_block *sb) { struct tracefs_inode *ti; + unsigned long flags; ti = kmem_cache_alloc(tracefs_inode_cachep, GFP_KERNEL); if (!ti) return NULL; + spin_lock_irqsave(&tracefs_inode_lock, flags); + list_add_rcu(&ti->list, &tracefs_inodes); + spin_unlock_irqrestore(&tracefs_inode_lock, flags); + return &ti->vfs_inode; } +static void tracefs_free_inode_rcu(struct rcu_head *rcu) +{ + struct tracefs_inode *ti; + + ti = container_of(rcu, struct tracefs_inode, rcu); + kmem_cache_free(tracefs_inode_cachep, ti); +} + static void tracefs_free_inode(struct inode *inode) { - kmem_cache_free(tracefs_inode_cachep, get_tracefs(inode)); + struct tracefs_inode *ti = get_tracefs(inode); + unsigned long flags; + + spin_lock_irqsave(&tracefs_inode_lock, flags); + list_del_rcu(&ti->list); + spin_unlock_irqrestore(&tracefs_inode_lock, flags); + + call_rcu(&ti->rcu, tracefs_free_inode_rcu); } static ssize_t default_read_file(struct file *file, char __user *buf, @@ -313,6 +340,8 @@ static int tracefs_apply_options(struct super_block *sb, bool remount) struct tracefs_fs_info *fsi = sb->s_fs_info; struct inode *inode = d_inode(sb->s_root); struct tracefs_mount_opts *opts = &fsi->mount_opts; + struct tracefs_inode *ti; + bool update_uid, update_gid; umode_t tmp_mode; /* @@ -332,6 +361,25 @@ static int tracefs_apply_options(struct super_block *sb, bool remount) if (!remount || opts->opts & BIT(Opt_gid)) inode->i_gid = opts->gid; + if (remount && (opts->opts & BIT(Opt_uid) || opts->opts & BIT(Opt_gid))) { + + update_uid = opts->opts & BIT(Opt_uid); + update_gid = opts->opts & BIT(Opt_gid); + + rcu_read_lock(); + list_for_each_entry_rcu(ti, &tracefs_inodes, list) { + if (update_uid) + ti->flags &= ~TRACEFS_UID_PERM_SET; + + if (update_gid) + ti->flags &= ~TRACEFS_GID_PERM_SET; + + if (ti->flags & TRACEFS_EVENT_INODE) + eventfs_remount(ti, update_uid, update_gid); + } + rcu_read_unlock(); + } + return 0; } @@ -398,7 +446,22 @@ static int tracefs_d_revalidate(struct dentry *dentry, unsigned int flags) return !(ei && ei->is_freed); } +static void tracefs_d_iput(struct dentry *dentry, struct inode *inode) +{ + struct tracefs_inode *ti = get_tracefs(inode); + + /* + * This inode is being freed and cannot be used for + * eventfs. Clear the flag so that it doesn't call into + * eventfs during the remount flag updates. The eventfs_inode + * gets freed after an RCU cycle, so the content will still + * be safe if the iteration is going on now. + */ + ti->flags &= ~TRACEFS_EVENT_INODE; +} + static const struct dentry_operations tracefs_dentry_operations = { + .d_iput = tracefs_d_iput, .d_revalidate = tracefs_d_revalidate, .d_release = tracefs_d_release, }; diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h index 15c26f9aaad4..29f0c999975b 100644 --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -11,8 +11,12 @@ enum { }; struct tracefs_inode { - struct inode vfs_inode; + union { + struct inode vfs_inode; + struct rcu_head rcu; + }; /* The below gets initialized with memset_after(ti, 0, vfs_inode) */ + struct list_head list; unsigned long flags; void *private; }; @@ -73,6 +77,7 @@ struct dentry *tracefs_end_creating(struct dentry *dentry); struct dentry *tracefs_failed_creating(struct dentry *dentry); struct inode *tracefs_get_inode(struct super_block *sb); +void eventfs_remount(struct tracefs_inode *ti, bool update_uid, bool update_gid); void eventfs_d_release(struct dentry *dentry); #endif /* _TRACEFS_INTERNAL_H */ -- 2.43.0

12 months

1
0
0 0

[PATCH v3 1/6] eventfs: Free all of the eventfs_inode after RCU

by Steven Rostedt

From: "Steven Rostedt (Google)" <rostedt(a)goodmis.org> The freeing of eventfs_inode via a kfree_rcu() callback. But the content of the eventfs_inode was being freed after the last kref. This is dangerous, as changes are being made that can access the content of an eventfs_inode from an RCU loop. Instead of using kfree_rcu() use call_rcu() that calls a function to do all the freeing of the eventfs_inode after a RCU grace period has expired. Cc: stable(a)vger.kernel.org Fixes: 43aa6f97c2d03 ("eventfs: Get rid of dentry pointers without refcounts") Signed-off-by: Steven Rostedt (Google) <rostedt(a)goodmis.org> --- fs/tracefs/event_inode.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index f5510e26f0f6..cc8b838bbe62 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -73,6 +73,21 @@ enum { #define EVENTFS_MODE_MASK (EVENTFS_SAVE_MODE - 1) +static void free_ei_rcu(struct rcu_head *rcu) +{ + struct eventfs_inode *ei = container_of(rcu, struct eventfs_inode, rcu); + struct eventfs_root_inode *rei; + + kfree(ei->entry_attrs); + kfree_const(ei->name); + if (ei->is_events) { + rei = get_root_inode(ei); + kfree(rei); + } else { + kfree(ei); + } +} + /* * eventfs_inode reference count management. * @@ -85,7 +100,6 @@ static void release_ei(struct kref *ref) { struct eventfs_inode *ei = container_of(ref, struct eventfs_inode, kref); const struct eventfs_entry *entry; - struct eventfs_root_inode *rei; WARN_ON_ONCE(!ei->is_freed); @@ -95,14 +109,7 @@ static void release_ei(struct kref *ref) entry->release(entry->name, ei->data); } - kfree(ei->entry_attrs); - kfree_const(ei->name); - if (ei->is_events) { - rei = get_root_inode(ei); - kfree_rcu(rei, ei.rcu); - } else { - kfree_rcu(ei, rcu); - } + call_rcu(&ei->rcu, free_ei_rcu); } static inline void put_ei(struct eventfs_inode *ei) -- 2.43.0

12 months

1
0
0 0

[PATCH v2] arm64/mm: pmd_mkinvalid() must handle swap pmds

by Ryan Roberts

__split_huge_pmd_locked() can be called for a present THP, devmap or (non-present) migration entry. It calls pmdp_invalidate() unconditionally on the pmdp and only determines if it is present or not based on the returned old pmd. But arm64's pmd_mkinvalid(), called by pmdp_invalidate(), unconditionally sets the PMD_PRESENT_INVALID flag, which causes future pmd_present() calls to return true - even for a swap pmd. Therefore any lockless pgtable walker could see the migration entry pmd in this state and start interpretting the fields (e.g. pmd_pfn()) as if it were present, leading to BadThings (TM). GUP-fast appears to be one such lockless pgtable walker. While the obvious fix is for core-mm to avoid such calls for non-present pmds (pmdp_invalidate() will also issue TLBI which is not necessary for this case either), all other arches that implement pmd_mkinvalid() do it in such a way that it is robust to being called with a non-present pmd. So it is simpler and safer to make arm64 robust too. This approach means we can even add tests to debug_vm_pgtable.c to validate the required behaviour. This is a theoretical bug found during code review. I don't have any test case to trigger it in practice. Cc: stable(a)vger.kernel.org Fixes: 53fa117bb33c ("arm64/mm: Enable THP migration") Signed-off-by: Ryan Roberts <ryan.roberts(a)arm.com> --- Hi all, v1 of this fix [1] took the approach of fixing core-mm to never call pmdp_invalidate() on a non-present pmd. But Zi Yan highlighted that only arm64 suffers this problem; all other arches are robust. So his suggestion was to instead make arm64 robust in the same way and add tests to validate it. Despite my stated reservations in the context of the v1 discussion, having thought on it for a bit, I now agree with Zi Yan. Hence this post. Andrew has v1 in mm-unstable at the moment, so probably the best thing to do is remove it from there and have this go in through the arm64 tree? Assuming there is agreement that this approach is right one. This applies on top of v6.9-rc5. Passes all the mm selftests on arm64. [1] https://lore.kernel.org/linux-mm/20240425170704.3379492-1-ryan.roberts@arm.… Thanks, Ryan arch/arm64/include/asm/pgtable.h | 12 +++++-- mm/debug_vm_pgtable.c | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h index afdd56d26ad7..7d580271a46d 100644 --- a/arch/arm64/include/asm/pgtable.h +++ b/arch/arm64/include/asm/pgtable.h @@ -511,8 +511,16 @@ static inline int pmd_trans_huge(pmd_t pmd) static inline pmd_t pmd_mkinvalid(pmd_t pmd) { - pmd = set_pmd_bit(pmd, __pgprot(PMD_PRESENT_INVALID)); - pmd = clear_pmd_bit(pmd, __pgprot(PMD_SECT_VALID)); + /* + * If not valid then either we are already present-invalid or we are + * not-present (i.e. none or swap entry). We must not convert + * not-present to present-invalid. Unbelievably, the core-mm may call + * pmd_mkinvalid() for a swap entry and all other arches can handle it. + */ + if (pmd_valid(pmd)) { + pmd = set_pmd_bit(pmd, __pgprot(PMD_PRESENT_INVALID)); + pmd = clear_pmd_bit(pmd, __pgprot(PMD_SECT_VALID)); + } return pmd; } diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c index 65c19025da3d..7e9c387d06b0 100644 --- a/mm/debug_vm_pgtable.c +++ b/mm/debug_vm_pgtable.c @@ -956,6 +956,65 @@ static void __init hugetlb_basic_tests(struct pgtable_debug_args *args) { } #endif /* CONFIG_HUGETLB_PAGE */ #ifdef CONFIG_TRANSPARENT_HUGEPAGE +#if !defined(__HAVE_ARCH_PMDP_INVALIDATE) && defined(CONFIG_ARCH_ENABLE_THP_MIGRATION) +static void __init swp_pmd_mkinvalid_tests(struct pgtable_debug_args *args) +{ + unsigned long max_swap_offset; + swp_entry_t swp_set, swp_clear, swp_convert; + pmd_t pmd_set, pmd_clear; + + /* + * See generic_max_swapfile_size(): probe the maximum offset, then + * create swap entry will all possible bits set and a swap entry will + * all bits clear. + */ + max_swap_offset = swp_offset(pmd_to_swp_entry(swp_entry_to_pmd(swp_entry(0, ~0UL)))); + swp_set = swp_entry((1 << MAX_SWAPFILES_SHIFT) - 1, max_swap_offset); + swp_clear = swp_entry(0, 0); + + /* Convert to pmd. */ + pmd_set = swp_entry_to_pmd(swp_set); + pmd_clear = swp_entry_to_pmd(swp_clear); + + /* + * Sanity check that the pmds are not-present, not-huge and swap entry + * is recoverable without corruption. + */ + WARN_ON(pmd_present(pmd_set)); + WARN_ON(pmd_trans_huge(pmd_set)); + swp_convert = pmd_to_swp_entry(pmd_set); + WARN_ON(swp_type(swp_set) != swp_type(swp_convert)); + WARN_ON(swp_offset(swp_set) != swp_offset(swp_convert)); + WARN_ON(pmd_present(pmd_clear)); + WARN_ON(pmd_trans_huge(pmd_clear)); + swp_convert = pmd_to_swp_entry(pmd_clear); + WARN_ON(swp_type(swp_clear) != swp_type(swp_convert)); + WARN_ON(swp_offset(swp_clear) != swp_offset(swp_convert)); + + /* Now invalidate the pmd. */ + pmd_set = pmd_mkinvalid(pmd_set); + pmd_clear = pmd_mkinvalid(pmd_clear); + + /* + * Since its a swap pmd, invalidation should effectively be a noop and + * the checks we already did should give the same answer. Check the + * invalidation didn't corrupt any fields. + */ + WARN_ON(pmd_present(pmd_set)); + WARN_ON(pmd_trans_huge(pmd_set)); + swp_convert = pmd_to_swp_entry(pmd_set); + WARN_ON(swp_type(swp_set) != swp_type(swp_convert)); + WARN_ON(swp_offset(swp_set) != swp_offset(swp_convert)); + WARN_ON(pmd_present(pmd_clear)); + WARN_ON(pmd_trans_huge(pmd_clear)); + swp_convert = pmd_to_swp_entry(pmd_clear); + WARN_ON(swp_type(swp_clear) != swp_type(swp_convert)); + WARN_ON(swp_offset(swp_clear) != swp_offset(swp_convert)); +} +#else +static void __init swp_pmd_mkinvalid_tests(struct pgtable_debug_args *args) { } +#endif /* !__HAVE_ARCH_PMDP_INVALIDATE && CONFIG_ARCH_ENABLE_THP_MIGRATION */ + static void __init pmd_thp_tests(struct pgtable_debug_args *args) { pmd_t pmd; @@ -982,6 +1041,8 @@ static void __init pmd_thp_tests(struct pgtable_debug_args *args) WARN_ON(!pmd_trans_huge(pmd_mkinvalid(pmd_mkhuge(pmd)))); WARN_ON(!pmd_present(pmd_mkinvalid(pmd_mkhuge(pmd)))); #endif /* __HAVE_ARCH_PMDP_INVALIDATE */ + + swp_pmd_mkinvalid_tests(args); } #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD -- 2.25.1

12 months

4
14
0 0

[PATCH] docs: stable-kernel-rules: fix typo sent->send

by Bird, Tim

Change 'sent' to 'send' Signed-off-by: Tim Bird <tim.bird(a)sony.com> --- Documentation/process/stable-kernel-rules.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/process/stable-kernel-rules.rst b/Documentation/process/stable-kernel-rules.rst index 1704f1c686d0..3178bef6fca3 100644 --- a/Documentation/process/stable-kernel-rules.rst +++ b/Documentation/process/stable-kernel-rules.rst @@ -78,7 +78,7 @@ in the sign-off area. Once the patch is mainlined it will be applied to the stable tree without anything else needing to be done by the author or subsystem maintainer. -To sent additional instructions to the stable team, use a shell-style inline +To send additional instructions to the stable team, use a shell-style inline comment: * To specify any additional patch prerequisites for cherry picking use the -- 2.25.1

12 months

5
4
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror May 2024