Re: [PATCH v3 36/50] functionfs: switch to simple_remove_by_name()

[Chris Mason <clm@meta.com>]

On 11/11/25 4:22 AM, Al Viro wrote:
> On Tue, Nov 11, 2025 at 07:53:16AM +0000, bot+bpf-ci@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;
}