Bug 221048 - minor() truncates device number to 32 bits, whereas dev_t type was extended to 64 bits
Summary: minor() truncates device number to 32 bits, whereas dev_t type was extended t...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Konstantin Belousov
URL: http://bugs.python.org/issue31044
Keywords: regression
Depends on:
Blocks:
 
Reported: 2017-07-27 14:30 UTC by Victor Stinner
Modified: 2018-06-08 04:41 UTC (History)
4 users (show)

See Also:


Attachments
Change major()/minor() to work with 64bit dev_t. (875 bytes, patch)
2017-07-28 12:42 UTC, Konstantin Belousov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Stinner 2017-07-27 14:30:09 UTC
A commit made at May, 23, changed dev_t type from 32 bits to 64 bits:

* https://reviews.freebsd.org/D10439
* https://github.com/freebsd/freebsd/commit/e75ba1d5c4c79376a78351c8544388491db49664#diff-12c2f5b61ce3b017a25144619d13ca66L108

The problem is that the userland minor() macro explicitly truncates to 32 bits:

#define	minor(x) ((int)((x)&0xffff00ff)) /* minor number */

https://github.com/freebsd/freebsd/blob/master/sys/sys/types.h#L373

64-bit dev_t broke a unit test Python:
http://bugs.python.org/issue31044

In short, Python checks that makedev(major(dev), minor(dev))==dev, whereas dev comes from stat.st_dev of a file created in the current directory (ex: ZFS filesystem for $HOME).

Example:

* stat() returns st_dev = 0xde4d0429ab
* major(0xde4d0429ab) returns 0x29
* minor(0xde4d0429ab) returns 0x4d0400ab

The problem is that minor() truncates 0xde00000000 high bits.
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2017-07-28 12:42:27 UTC
Created attachment 184797 [details]
Change major()/minor() to work with 64bit dev_t.

Since traditional types for the macros values are int, I removed the cookie trick and just split the dev_t at the word boundary.
Comment 2 commit-hook freebsd_committer freebsd_triage 2017-08-02 10:15:07 UTC
A commit references this bug:

Author: kib
Date: Wed Aug  2 10:14:18 UTC 2017
New revision: 321920
URL: https://svnweb.freebsd.org/changeset/base/321920

Log:
  Change major()/minor() to work with 64bit dev_t.

  Since traditional types for the macros values are int, remove the
  cookie trick and just split the dev_t at the word boundary.

  Reported by:	Victor Stinner <victor.stinner@gmail.com>
  PR:	221048
  Sponsored by:	The FreeBSD Foundation

Changes:
  head/sys/sys/types.h
Comment 3 commit-hook freebsd_committer freebsd_triage 2017-08-03 03:44:33 UTC
A commit references this bug:

Author: ngie
Date: Thu Aug  3 03:43:41 UTC 2017
New revision: 321967
URL: https://svnweb.freebsd.org/changeset/base/321967

Log:
  Chase r321920 and r321930 (dev_t being widened)

  The layout of st_rdev has changed after this commit, and assumptions made
  in the NetBSD tests are no longer valid. Change the hardcoded assumed
  values to account for the fact that major/minor are now represented by
  64 bits as opposed to the less precise legacy precision of 16 bits.

  PR:	221048
  Relnotes: st_rdev layout changed; warning about impact of r321920 to
  	  downstream consumers

Changes:
  head/tests/sys/fs/tmpfs/Makefile
Comment 4 vali gholami 2017-12-17 07:15:11 UTC
MARKED AS SPAM
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:05 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2018-06-08 04:41:48 UTC
base r321920 does not appear to need MFC'ing as the 64-bit inode project (the originating code for the regression) is for FreeBSD CURRENT (12.x) only. Close (resolved) accordingly.

@kib Please re-open if this is not the case.