Bug 204358 - zfs loader zfs_probe_args secsz is too small, causing memory corruption
Summary: zfs loader zfs_probe_args secsz is too small, causing memory corruption
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Allan Jude
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-11-07 13:20 UTC by Toomas Soome
Modified: 2016-04-02 16:58 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Toomas Soome 2015-11-07 13:20:41 UTC
sys/boot/zfs/zfs.c has probe args structure including uint16_t secsz variable for media sector size; its used as an argument for ioctl() at line 484:

ioctl(pa.fd, DIOCGSECTORSIZE, &pa.secsz);

however, this ioctl is expecting 32bit data (u_int *) and therefore this ioctl will overwrite and corrupt 16bits of memory.

other use cases seem to use correct u_int type for secsz.

for fix the following fix should be sufficient.
tsoome@beastie:/code/freebsd/head/sys/boot/zfs$ diff -u zfs.c.orig zfs.c
--- zfs.c.orig  N apr 16 14:49:00 2015
+++ zfs.c       L nov  7 15:13:55 2015
@@ -399,7 +399,7 @@
        int             fd;
        const char      *devname;
        uint64_t        *pool_guid;
-       uint16_t        secsz;
+       u_int           secsz;
 };
 
 static int
Comment 1 Allan Jude freebsd_committer 2016-01-07 01:45:19 UTC
https://reviews.freebsd.org/D4811
Comment 2 commit-hook freebsd_committer 2016-01-11 15:36:27 UTC
A commit references this bug:

Author: allanjude
Date: Mon Jan 11 15:35:30 UTC 2016
New revision: 293661
URL: https://svnweb.freebsd.org/changeset/base/293661

Log:
  DIOCGSECTORSIZE expects to write to a u_int, but struct zfs_probe_args
  member secsz was a uint16_t

  sys/boot/zfs/zfs.c has a probe args structure member, secsz, that is a
  uint16_t for media sector size; it is used as an argument for ioctl()
  at line 484. however, this ioctl writes 32 bits of data (u_int *) and
  therefore this ioctl will overwrite and corrupt 16 bits of memory.
  other use cases seem to use correct u_int type for secsz.

  PR:		204358
  Submitted by:	Toomas Soome <tsoome at me.com>
  Reviewed by:	asomers, delphij, smh
  MFC after:	5 days
  Differential Revision:	https://reviews.freebsd.org/D4811

Changes:
  head/sys/boot/zfs/zfs.c
Comment 3 Andriy Gapon freebsd_committer 2016-04-02 16:57:23 UTC
Assign to the committer.