Re: [PATCH] md/raid10: fix divide-by-zero in setup_geo() with zero far_copies

Junrui Luo <[email protected]>
Newsgroups gmane.linux.kernel.stable,gmane.linux.raid,gmane.linux.kernel
Message-ID <[email protected]>
Hi Paul,

Thank you for the review.

On Thu, Apr 16, 2026 at 08:17:26AM +0200, Paul Menzel wrote:
> Why also `nc` and not just `fc`?

nc and fc are documented as "must be at least one" (raid10.c
line 47), it seemed cleaner to reject both together.

> It’d be great, if you documented the command how to create such a layout.

Here is a reproducer that triggers the divide-by-zero

  for i in 0 1 2 3; do
    dd if=/dev/zero of=/tmp/loop$i bs=1M count=64
    losetup /dev/loop$i /tmp/loop$i
  done

  gcc -o raid10_poc raid10_poc.c
  ./raid10_poc

```
  #include <stdio.h>
  #include <stdlib.h>
  #include <fcntl.h>
  #include <unistd.h>
  #include <string.h>
  #include <sys/ioctl.h>
  #include <sys/stat.h>
  #include <sys/sysmacros.h>
  #include <linux/major.h>
  #include <linux/raid/md_u.h>

  int main(void)
  {
  	int fd, i;
  	mdu_array_info_t array;
  	mdu_disk_info_t disk;

  	mknod("/dev/md0", S_IFBLK | 0600, makedev(9, 0));

  	fd = open("/dev/md0", O_RDWR);
  	if (fd < 0) {
  		perror("open /dev/md0");
  		return 1;
  	}

  	memset(&array, 0, sizeof(array));
  	array.major_version = 1;
  	array.minor_version = 2;
  	array.level = 10;
  	array.layout = 0x20000;
  	array.raid_disks = 4;
  	array.chunk_size = 65536;

  	if (ioctl(fd, SET_ARRAY_INFO, &array) < 0) {
  		perror("SET_ARRAY_INFO");
  		return 1;
  	}

  	for (i = 0; i < 4; i++) {
  		memset(&disk, 0, sizeof(disk));
  		disk.number = i;
  		disk.raid_disk = i;
  		disk.state = (1 << 1) | (1 << 2);
  		disk.major = 7;
  		disk.minor = i;
  		if (ioctl(fd, ADD_NEW_DISK, &disk) < 0) {
  			perror("ADD_NEW_DISK");
  			return 1;
  		}
  	}

  	/* triggers setup_conf() -> setup_geo() -> disks/fc with fc=0 */
  	ioctl(fd, RUN_ARRAY, NULL);

  	close(fd);
  	return 0;
  }
```

> I’d also print a warning, so the user knows, what was wrong:
> 
>     pr_warn(md/raid10:%s: near and far copies need to be greater than 0,
> mdname(mddev));
 
With this fix, nc=0 or fc=0 returns -1, which hits the `copies < 2`
check and prints the existing warning. Adding another pr_warn inside
setup_geo() would be inconsistent with the other `return -1` paths in
that function, which all silently return -1 and let the caller report.
Adding a pr_warn for this case alone would be inconsistent; doing it
properly would mean adding warnings to all the return -1 paths, which
is a larger change better done separately.

Thanks,
Junrui Luo
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.