[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] nfsd: clear acl_access/acl_default after releasing them #633
+4
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commit 7faf14a upstream. If getting acl_default fails, acl_access and acl_default will be released simultaneously. However, acl_access will still retain a pointer pointing to the released posix_acl, which will trigger a WARNING in nfs3svc_release_getacl like this: ------------[ cut here ]------------ refcount_t: underflow; use-after-free. WARNING: CPU: 26 PID: 3199 at lib/refcount.c:28 refcount_warn_saturate+0xb5/0x170 Modules linked in: CPU: 26 UID: 0 PID: 3199 Comm: nfsd Not tainted 6.12.0-rc6-00079-g04ae226af01f-dirty #8 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.1-2.fc37 04/01/2014 RIP: 0010:refcount_warn_saturate+0xb5/0x170 Code: cc cc 0f b6 1d b3 20 a5 03 80 fb 01 0f 87 65 48 d8 00 83 e3 01 75 e4 48 c7 c7 c0 3b 9b 85 c6 05 97 20 a5 03 01 e8 fb 3e 30 ff <0f> 0b eb cd 0f b6 1d 8a3 RSP: 0018:ffffc90008637cd8 EFLAGS: 00010282 RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff83904fde RDX: dffffc0000000000 RSI: 0000000000000008 RDI: ffff88871ed36380 RBP: ffff888158beeb40 R08: 0000000000000001 R09: fffff520010c6f56 R10: ffffc90008637ab7 R11: 0000000000000001 R12: 0000000000000001 R13: ffff888140e77400 R14: ffff888140e77408 R15: ffffffff858b42c0 FS: 0000000000000000(0000) GS:ffff88871ed00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000562384d32158 CR3: 000000055cc6a000 CR4: 00000000000006f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> ? refcount_warn_saturate+0xb5/0x170 ? __warn+0xa5/0x140 ? refcount_warn_saturate+0xb5/0x170 ? report_bug+0x1b1/0x1e0 ? handle_bug+0x53/0xa0 ? exc_invalid_op+0x17/0x40 ? asm_exc_invalid_op+0x1a/0x20 ? tick_nohz_tick_stopped+0x1e/0x40 ? refcount_warn_saturate+0xb5/0x170 ? refcount_warn_saturate+0xb5/0x170 nfs3svc_release_getacl+0xc9/0xe0 svc_process_common+0x5db/0xb60 ? __pfx_svc_process_common+0x10/0x10 ? __rcu_read_unlock+0x69/0xa0 ? __pfx_nfsd_dispatch+0x10/0x10 ? svc_xprt_received+0xa1/0x120 ? xdr_init_decode+0x11d/0x190 svc_process+0x2a7/0x330 svc_handle_xprt+0x69d/0x940 svc_recv+0x180/0x2d0 nfsd+0x168/0x200 ? __pfx_nfsd+0x10/0x10 kthread+0x1a2/0x1e0 ? kthread+0xf4/0x1e0 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x34/0x60 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 </TASK> Kernel panic - not syncing: kernel: panic_on_warn set ... Clear acl_access/acl_default after posix_acl_release is called to prevent UAF from being triggered. Fixes: a257cdd ("[PATCH] NFSD: Add server support for NFSv3 ACLs.") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/all/20241107014705.2509463-1-lilingfeng@huaweicloud.com/ Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> Reviewed-by: Rick Macklem <rmacklem@uoguelph.ca> Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewer's Guide by SourceryThe pull request updates the kernel base to version 6.6.79, includes a fix for LoongArch idle VS timer enqueue, and adds a bugfix for serial 8250 FIFO underflow on flush. Updated class diagram for Steam DeviceclassDiagram
class steam_device {
+list_head list
+spinlock_t lock
+hid_device* hdev
+hid_device* client_hdev
+mutex report_mutex
+bool client_opened
+input_dev __rcu* input
+input_dev __rcu* sensors
+unsigned long quirks
+work_struct work_connect
+bool connected
+power_supply __rcu* battery
+u8 battery_charge
+u16 voltage
+delayed_work mode_switch
+bool did_mode_switch
+bool gamepad_mode
+work_struct rumble_work
+u16 rumble_left
+u16 rumble_right
+unsigned int sensor_timestamp_us
+work_struct unregister_work
}
note for steam_device "Added new attributes: sensors, mode_switch, did_mode_switch, gamepad_mode, sensor_timestamp_us, unregister_work"
Updated class diagram for RAID5 ConfigurationclassDiagram
class r5conf {
+sector_t reshape_progress
+sector_t reshape_safe
+spinlock_t device_lock
+int chunk_sectors
+int raid_disks
+int max_degraded
+int prev_chunk_sectors
+int previous_raid_disks
}
note for r5conf "Added new attributes: reshape_safe, prev_chunk_sectors, previous_raid_disks"
Updated class diagram for BCM Kona GPIO BankclassDiagram
class bcm_kona_gpio_bank {
+int id
+int irq
+u8 gpio_unlock_count[GPIO_PER_BANK]
+bcm_kona_gpio* kona_gpio
}
note for bcm_kona_gpio_bank "Added new attribute: gpio_unlock_count"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin-ci-robot
requested review from
Ink-Paper,
hello666888999 and
huangbibo
Member
Author
|
/approve because of #629 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Avenger-285714 - I've reviewed your changes - here's some feedback:
Overall Comments:
- The enum redefinition for String attribute identifiers is misspelled as 'idenitifiers'.
- The steam_mode_switch_cb function should check steam->did_mode_switch before scheduling the work to avoid potential race conditions.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 3 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| * | ||
| * Emits broadcast ELP messages in regular intervals. | ||
| */ | ||
| static void batadv_v_elp_periodic_work(struct work_struct *work) |
There was a problem hiding this comment.
issue (complexity): Consider using a two-phase approach with a preallocated stack array to collect neighbor pointers instead of dynamic allocation and list management.
Consider replacing the temporary allocation and list‐management with a simple two‐phase approach that collects neighbor pointers in a preallocated (stack) array. For example, if you can bound the number of neighbors, you might do:
```c
#define MAX_NEIGHBORS 128
static void batadv_v_elp_periodic_work(struct work_struct *work)
{
struct batadv_hardif_neigh_node *neighbors[MAX_NEIGHBORS];
unsigned int count = 0;
struct batadv_hardif_neigh_node *hardif_neigh;
struct batadv_hard_iface *hard_iface;
struct batadv_hard_iface_bat_v *bat_v;
struct batadv_priv *bat_priv;
// ... other declarations
bat_v = container_of(work, struct batadv_hard_iface_bat_v, elp_wq.work);
hard_iface = container_of(bat_v, struct batadv_hard_iface, bat_v);
bat_priv = netdev_priv(hard_iface->soft_iface);
if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING)
goto out;
if (hard_iface->if_status == BATADV_IF_NOT_IN_USE)
goto out;
// ... e.g. sending broadcast packet, etc.
rcu_read_lock();
list_for_each_entry(hardif_neigh, &hard_iface->neighbors, list) {
if (!kref_get_unless_zero(&hardif_neigh->refcount))
continue;
if (count < MAX_NEIGHBORS)
neighbors[count++] = hardif_neigh;
else
batadv_hardif_neigh_put(hardif_neigh);
}
rcu_read_unlock();
for (unsigned int i = 0; i < count; i++) {
batadv_v_elp_throughput_metric_update(neighbors[i]);
batadv_hardif_neigh_put(neighbors[i]);
}
restart_timer:
batadv_v_elp_start_timer(hard_iface);
out:
return;
}This change avoids dynamic memory allocation (kzalloc) and explicit list handling while keeping all functionality intact. Adjust MAX_NEIGHBORS as needed.
drivers/mmc/host/mtk-sd.c
Outdated
| host->top_base + EMMC50_PAD_DS_TUNE); | ||
| else | ||
| writel(host->hs400_ds_delay, host->base + PAD_DS_TUNE); | ||
| if (host->top_base) { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring duplicated conditional branches into a helper function to simplify the code.
Consider refactoring the duplicated “if (host->top_base)” branches into a helper function that performs the common tuning steps. For example, you could add a static helper that accepts the base pointer and proper tuning field definitions. This way you reduce nested conditionals while preserving functionality. One approach might be:
static void set_hs400_tune(void __iomem *reg_base, u32 ds_dly3, u32 ds_delay, u32 dly3_field)
{
if (ds_dly3)
sdr_set_field(reg_base, dly3_field, ds_dly3);
if (ds_delay)
writel(ds_delay, reg_base);
}Then in your tuning prepare functions you can simply do:
if (host->top_base)
set_hs400_tune(host->top_base + EMMC50_PAD_DS_TUNE,
host->hs400_ds_dly3,
host->hs400_ds_delay,
PAD_DS_DLY3);
else
set_hs400_tune(host->base + PAD_DS_TUNE,
host->hs400_ds_dly3,
host->hs400_ds_delay,
PAD_DS_TUNE_DLY3);A similar strategy can be applied in msdc_execute_hs400_tuning for the duplicated logic. This refactoring helps to reduce deep nesting and duplicated code, making the code easier to read and maintain without changing its behavior.
drivers/usb/class/cdc-acm.c
Outdated
| usb_mark_last_busy(acm->dev); | ||
|
|
||
| if (acm->nb_index) | ||
| if (acm->nb_index == 0) { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the first-chunk handling into a helper function to simplify the control flow.
Consider refactoring the first-chunk handling into a small helper to isolate and document the extra logic. For example, extract something like this:
static int handle_first_chunk(struct acm *acm, struct urb *urb,
struct usb_cdc_notification **dr)
{
unsigned int current_size = urb->actual_length;
if (current_size < sizeof(struct usb_cdc_notification)) {
dev_dbg(&acm->control->dev, "urb too short\n");
return -EINVAL; // or goto exit in caller
}
*dr = urb->transfer_buffer;
return 0;
}Then in acm_ctrl_irq(), simplify the branch:
if (acm->nb_index == 0) {
if (handle_first_chunk(acm, urb, &dr) < 0)
goto exit;
} else {
dr = (struct usb_cdc_notification *)acm->notification_buffer;
}This isolates the special case, reducing nesting and making the overall control flow clearer.
Avenger-285714
changed the title
deepin pr auto review代码审查意见：
综上所述，代码修改是合理的，没有发现需要立即修改的问题。 |
Member
Author
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Avenger-285714 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Avenger-285714
changed the title
Avenger-285714
merged commit 057c879
into
deepin-community:linux-6.6.y
4 of 5 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
commit 7faf14a upstream.
If getting acl_default fails, acl_access and acl_default will be released
simultaneously. However, acl_access will still retain a pointer pointing
to the released posix_acl, which will trigger a WARNING in
nfs3svc_release_getacl like this:
------------[ cut here ]------------
refcount_t: underflow; use-after-free.
WARNING: CPU: 26 PID: 3199 at lib/refcount.c:28
refcount_warn_saturate+0xb5/0x170
Modules linked in:
CPU: 26 UID: 0 PID: 3199 Comm: nfsd Not tainted
6.12.0-rc6-00079-g04ae226af01f-dirty #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.16.1-2.fc37 04/01/2014
RIP: 0010:refcount_warn_saturate+0xb5/0x170
Code: cc cc 0f b6 1d b3 20 a5 03 80 fb 01 0f 87 65 48 d8 00 83 e3 01 75
e4 48 c7 c7 c0 3b 9b 85 c6 05 97 20 a5 03 01 e8 fb 3e 30 ff <0f> 0b eb
cd 0f b6 1d 8a3
RSP: 0018:ffffc90008637cd8 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff83904fde
RDX: dffffc0000000000 RSI: 0000000000000008 RDI: ffff88871ed36380
RBP: ffff888158beeb40 R08: 0000000000000001 R09: fffff520010c6f56
R10: ffffc90008637ab7 R11: 0000000000000001 R12: 0000000000000001
R13: ffff888140e77400 R14: ffff888140e77408 R15: ffffffff858b42c0
FS: 0000000000000000(0000) GS:ffff88871ed00000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000562384d32158 CR3: 000000055cc6a000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
? refcount_warn_saturate+0xb5/0x170
? __warn+0xa5/0x140
? refcount_warn_saturate+0xb5/0x170
? report_bug+0x1b1/0x1e0
? handle_bug+0x53/0xa0
? exc_invalid_op+0x17/0x40
? asm_exc_invalid_op+0x1a/0x20
? tick_nohz_tick_stopped+0x1e/0x40
? refcount_warn_saturate+0xb5/0x170
? refcount_warn_saturate+0xb5/0x170
nfs3svc_release_getacl+0xc9/0xe0
svc_process_common+0x5db/0xb60
? __pfx_svc_process_common+0x10/0x10
? __rcu_read_unlock+0x69/0xa0
? __pfx_nfsd_dispatch+0x10/0x10
? svc_xprt_received+0xa1/0x120
? xdr_init_decode+0x11d/0x190
svc_process+0x2a7/0x330
svc_handle_xprt+0x69d/0x940
svc_recv+0x180/0x2d0
nfsd+0x168/0x200
? __pfx_nfsd+0x10/0x10
kthread+0x1a2/0x1e0
? kthread+0xf4/0x1e0
? __pfx_kthread+0x10/0x10
ret_from_fork+0x34/0x60
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
Kernel panic - not syncing: kernel: panic_on_warn set ...
Clear acl_access/acl_default after posix_acl_release is called to prevent
UAF from being triggered.
Fixes: Avenger-285714@a257cdd ("[PATCH] NFSD: Add server support for NFSv3 ACLs.")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20241107014705.2509463-1-lilingfeng@huaweicloud.com/
Signed-off-by: Li Lingfeng lilingfeng3@huawei.com
Reviewed-by: Rick Macklem rmacklem@uoguelph.ca
Reviewed-by: Jeff Layton jlayton@kernel.org
Signed-off-by: Chuck Lever chuck.lever@oracle.com
Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org