[PATCH v3 36/50] functionfs: switch to simple_remove_by_name()
Chris Mason
clm at meta.com
Wed Nov 12 14:44:26 AEDT 2025
On 11/11/25 4:22 AM, Al Viro wrote:
> On Tue, Nov 11, 2025 at 07:53:16AM +0000, bot+bpf-ci at kernel.org wrote:
>
>> When ffs_epfiles_create() calls ffs_epfiles_destroy(epfiles, i - 1) after
>> the first ffs_sb_create_file() call fails (when i=1), it passes count=0.
>> The initialization loop starts at i=1, so epfiles[0].ffs is never
>> initialized.
>
> Incorrect. The loop in question is
>
> epfile = epfiles;
> for (i = 1; i <= count; ++i, ++epfile) {
> epfile->ffs = ffs;
> mutex_init(&epfile->mutex);
> mutex_init(&epfile->dmabufs_mutex);
> INIT_LIST_HEAD(&epfile->dmabufs);
> if (ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR)
> sprintf(epfile->name, "ep%02x", ffs->eps_addrmap[i]);
> else
> sprintf(epfile->name, "ep%u", i);
> err = ffs_sb_create_file(ffs->sb, epfile->name,
> epfile, &ffs_epfile_operations);
> if (err) {
> ffs_epfiles_destroy(epfiles, i - 1);
> return err;
> }
> }
>
> and invariant maintained through the loop is epfile == epfiles + (i - 1).
> We start with i == 1 and epfile == epfiles, modify neither variable in
> the loop body and increment both i and epfile by the same amount in
> the step.
>
> In other words, on the first pass through the loop we access epfiles[0],
> not epfiles[1]. Granted, the loop could've been more idiomatic, but
> it is actually correct.
AI was getting confused about epfile vs epfiles and didn't realize they
were pointing to the same memory. So I put some changes into the prompt
to sort that out, and it found a different variation on this same
complaint.
We're wandering into fuzzing territory here, and I honestly have no idea
if this is a valid use of any of this code, but AI managed to make a
repro that crashes only after your patch. So, I'll let you decide.
The new review:
Can this dereference ZERO_SIZE_PTR when eps_count is 0?
When ffs->eps_count is 0, ffs_epfiles_create() calls kcalloc(0, ...) which
returns ZERO_SIZE_PTR (0x10). The loop never executes so epfiles[0].ffs is
never initialized. Later, cleanup paths (ffs_data_closed and ffs_data_clear)
check if (epfiles) which is true for ZERO_SIZE_PTR, and call
ffs_epfiles_destroy(epfiles, 0).
In the old code, the for loop condition prevented any dereferences when
count=0. In the new code, "root = epfile->ffs->sb->s_root" dereferences
epfile before checking count, which would fault on ZERO_SIZE_PTR.
And the crash:
[ 21.714645] BUG: kernel NULL pointer dereference, address: 0000000000000030
[ 21.714764] #PF: supervisor read access in kernel mode
[ 21.714851] #PF: error_code(0x0000) - not-present page
[ 21.714968] PGD 10abe6067 P4D 10c0fa067 PUD 10864e067 PMD 0
[ 21.715155] Oops: Oops: 0000 [#1] SMP
[ 21.715226] CPU: 15 UID: 0 PID: 1071 Comm: test_ffs_crash Tainted: G E 6.18.0-rc4-g2b3cd169d144 #9 NONE
[ 21.715404] Tainted: [E]=UNSIGNED_MODULE
[ 21.715468] Hardware name: Red Hat KVM, BIOS 1.16.3-4.el9 04/01/2014
[ 21.715583] RIP: 0010:ffs_epfiles_destroy+0xe/0x70
[ 21.715681] Code: 4d ff ff ff 31 ff b8 01 00 00 00 eb 97 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 0f 1f 44 00 00 41 55 41 54 49 89 fc 55 53 <48> 8b 47 20 48 8b 80 50 01 00 00 48 8b 68 68 85 f6 74 3a 89 f6 48
[ 21.716055] RSP: 0018:ffa00000024cbe60 EFLAGS: 00010202
[ 21.716167] RAX: 0000000000000246 RBX: ff1100011ae81a00 RCX: ff11000117810540
[ 21.716286] RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000000000000010
[ 21.716461] RBP: ff1100011ae81a28 R08: ff1100010c1f7ac0 R09: ff11000111499ea0
[ 21.716568] R10: 0000000000000008 R11: 0000000000000000 R12: 0000000000000010
[ 21.716697] R13: ff11000111499ea0 R14: ff1100010ca5d260 R15: 0000000000000000
[ 21.716828] FS: 00007fdb2dbd6740(0000) GS:ff1100089ae74000(0000) knlGS:0000000000000000
[ 21.716992] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 21.717149] CR2: 0000000000000030 CR3: 000000010a2bb004 CR4: 0000000000771ef0
[ 21.717296] PKRU: 55555554
[ 21.717347] Call Trace:
[ 21.717390] <TASK>
[ 21.717434] ffs_data_clear+0xbb/0x140
[ 21.717496] ffs_data_closed+0x8e/0x1d0
[ 21.717565] ffs_ep0_release+0xe/0x20
[ 21.717639] __fput+0xdd/0x2a0
[ 21.717702] __x64_sys_close+0x39/0x70
[ 21.717768] do_syscall_64+0x5d/0x920
[ 21.717873] entry_SYSCALL_64_after_hwframe+0x4b/0x53
And the repro/fuzzer:
/*
* Test program to reproduce FunctionFS crash with eps_count=0
*
* This program creates a USB gadget with no endpoints (only EP0),
* which triggers the ZERO_SIZE_PTR dereference bug in ffs_epfiles_destroy().
*
* Setup:
* mount -t configfs none /sys/kernel/config
* cd /sys/kernel/config/usb_gadget
* mkdir g1 && cd g1
* echo 0x1d6b > idVendor
* echo 0x0104 > idProduct
* mkdir strings/0x409
* echo "1234567890" > strings/0x409/serialnumber
* echo "Manufacturer" > strings/0x409/manufacturer
* echo "Product" > strings/0x409/product
* mkdir configs/c.1
* mkdir configs/c.1/strings/0x409
* echo "Config 1" > configs/c.1/strings/0x409/configuration
* mkdir functions/ffs.test
* mkdir -p /dev/usb-ffs/test
* mount -t functionfs test /dev/usb-ffs/test
*
* Run:
* gcc -o test_ffs_crash test_ffs_crash.c
* ./test_ffs_crash
*
* Expected result on buggy kernel:
* Kernel crash when the program exits (cleanup calls ffs_epfiles_destroy
* with count=0, dereferences ZERO_SIZE_PTR)
*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdint.h>
#include <errno.h>
#define FUNCTIONFS_DESCRIPTORS_MAGIC_V2 3
#define FUNCTIONFS_STRINGS_MAGIC 2
/* FunctionFS flags */
#define FUNCTIONFS_HAS_FS_DESC (1 << 0)
#define FUNCTIONFS_HAS_HS_DESC (1 << 1)
/* USB descriptor types */
#define USB_DT_INTERFACE 0x04
struct usb_interface_descriptor {
uint8_t bLength;
uint8_t bDescriptorType;
uint8_t bInterfaceNumber;
uint8_t bAlternateSetting;
uint8_t bNumEndpoints; /* 0 - no endpoints! */
uint8_t bInterfaceClass;
uint8_t bInterfaceSubClass;
uint8_t bInterfaceProtocol;
uint8_t iInterface;
} __attribute__((packed));
struct ffs_descriptors {
uint32_t magic;
uint32_t length;
uint32_t flags;
uint32_t fs_count; /* Count of FS descriptors */
uint32_t hs_count; /* Count of HS descriptors */
/* Followed by descriptors */
} __attribute__((packed));
struct ffs_strings {
uint32_t magic;
uint32_t length;
uint32_t str_count;
uint32_t lang_count;
} __attribute__((packed));
int main(void)
{
int ep0_fd;
struct {
struct ffs_descriptors header;
struct usb_interface_descriptor fs_intf;
struct usb_interface_descriptor hs_intf;
} __attribute__((packed)) descs;
int ret;
printf("Opening /dev/usb-ffs/test/ep0...\n");
ep0_fd = open("/dev/usb-ffs/test/ep0", O_RDWR);
if (ep0_fd < 0) {
perror("open");
fprintf(stderr, "\nMake sure to setup configfs first:\n");
fprintf(stderr, "See comments at top of source file\n");
return 1;
}
printf("Writing descriptors with interface but NO endpoints (eps_count=0)...\n");
/* Build descriptor structure with interface descriptor but no endpoint descriptors */
memset(&descs, 0, sizeof(descs));
/* Header */
descs.header.magic = FUNCTIONFS_DESCRIPTORS_MAGIC_V2;
descs.header.length = sizeof(descs);
descs.header.flags = FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC;
descs.header.fs_count = 1; /* 1 descriptor for full-speed */
descs.header.hs_count = 1; /* 1 descriptor for high-speed */
/* Full-speed interface descriptor - NO endpoints! */
descs.fs_intf.bLength = sizeof(struct usb_interface_descriptor);
descs.fs_intf.bDescriptorType = USB_DT_INTERFACE;
descs.fs_intf.bInterfaceNumber = 0;
descs.fs_intf.bAlternateSetting = 0;
descs.fs_intf.bNumEndpoints = 0; /* KEY: No endpoints! */
descs.fs_intf.bInterfaceClass = 0xff; /* Vendor specific */
descs.fs_intf.bInterfaceSubClass = 0;
descs.fs_intf.bInterfaceProtocol = 0;
descs.fs_intf.iInterface = 0; /* No string descriptor */
/* High-speed interface descriptor - also NO endpoints! */
descs.hs_intf = descs.fs_intf;
ret = write(ep0_fd, &descs, sizeof(descs));
if (ret < 0) {
perror("write descriptors");
close(ep0_fd);
return 1;
}
printf("Wrote %d bytes of descriptors (interface with bNumEndpoints=0)\n", ret);
printf("Writing strings...\n");
/* Write strings with 1 language, 1 empty string */
struct {
struct ffs_strings header;
uint16_t lang;
char str1[1]; /* Empty string (just null terminator) */
} __attribute__((packed)) strings_data;
memset(&strings_data, 0, sizeof(strings_data));
strings_data.header.magic = FUNCTIONFS_STRINGS_MAGIC;
strings_data.header.length = sizeof(strings_data);
strings_data.header.str_count = 1; /* 1 string */
strings_data.header.lang_count = 1; /* 1 language */
strings_data.lang = 0x0409; /* English */
strings_data.str1[0] = '\0'; /* Empty string */
ret = write(ep0_fd, &strings_data, sizeof(strings_data));
if (ret < 0) {
perror("write strings");
close(ep0_fd);
return 1;
}
printf("Wrote %d bytes of strings\n", ret);
/* Closing the file will trigger cleanup path which calls
* ffs_data_closed() -> ffs_epfiles_destroy(ZERO_SIZE_PTR, 0) and crashes */
close(ep0_fd);
return 0;
}
More information about the Linuxppc-dev
mailing list