Bug 229977 - geom/gpart does not care about GPT size, silently creates partitions that won't be persisted after reboot
Summary: geom/gpart does not care about GPT size, silently creates partitions that won...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Marcel Moolenaar
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-07-23 12:03 UTC by Val Packett
Modified: 2019-03-23 03:10 UTC (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Val Packett 2018-07-23 12:03:01 UTC
So I've been trying to depenguinate a VPS that doesn't allow additional drives to be mounted. pivot_root into a tmpfs on Linux, dd the FreeBSD memstick image onto the disk, reboot into the installer, partition using shell, gpart recover (to move backup GPT to the end), gpart add, zpool create, etc. etc., install, reboot. Oops, no ZFS pools found. Booting into the installer again, and that third GPT partition I created simply does not exist!

Turns out the partition table size is limited to 2 entries.

Linux's gdisk utility is smart: trying to create a partition results in an error: "No table partition entries left".

While geom/gpart did not check the size and just created a partition that actually silently was not saved!!

Using expert mode in gdisk, I resized the partition table from 2 entries to 4 (using 128 results in "Main partition table overlaps the first partition by 31 blocks! Try reducing the partition table size by 124 entries.", but hey, 4 is enough for the VPS.) And everything worked fine.

Other people have stumbled upon this before: https://lists.freebsd.org/pipermail/freebsd-questions/2018-January/280562.html
Comment 1 Emmanuel Vadot freebsd_committer freebsd_triage 2018-07-23 17:30:20 UTC
Can you paste output of gpart show at each step ?

GPT can hold way more than 2 partitions.
Comment 2 Rodney W. Grimes freebsd_committer freebsd_triage 2018-07-23 17:47:14 UTC
(In reply to Emmanuel Vadot from comment #1)

The key word here is *can*, but part of a gpt header is the number of entries in the gpt, which typically defaults to 128, but can contain other values.

I suspect some tool wrote a gpt header with offset 80 == 2.
Comment 3 Emmanuel Vadot freebsd_committer freebsd_triage 2018-07-23 17:49:55 UTC
(In reply to Rodney W. Grimes from comment #2)

Yes, that's what I want to confirm.
I'm gonna have access to a machine there (scaleway) thanks to some friends who work there.
We might not do the right thing when such small gpt table
Comment 4 Val Packett 2018-07-23 17:59:38 UTC
(In reply to Rodney W. Grimes from comment #2)

I don't think that's Scaleway specific.

The GPT in question comes from this memstick image:

https://download.freebsd.org/ftp/snapshots/arm64/aarch64/ISO-IMAGES/12.0/FreeBSD-12.0-CURRENT-arm64-aarch64-20180719-r336479-mini-memstick.img.xz

The memstick is not *intended* to be extended with other partitions, so I guess that's understandable!

On the other hand, it's not like the amount of space a larger GPT takes is of any significance, the installer partition is over 500 megs :D

The actual problem is indeed that gpart is not doing the right thing when running out of available partition table entries.
Comment 5 Emmanuel Vadot freebsd_committer freebsd_triage 2018-07-23 18:02:42 UTC
(In reply to Greg V from comment #4)

Oh !!! I didn't understood that you were dealing with the memstick image...
So the memstick is created with mkimg, we can start by changing it so more partition space is allocated.
Then gpart need love too.
Comment 6 Ed Maste freebsd_committer freebsd_triage 2018-07-24 14:05:30 UTC
It looks like gpart creates a GPT with a minimum of 128 entries, we probably want to have mkimg do the same. Possibly with a command-line flag to override for cases e.g. where the user is trying to create a minimal-sized image.
Comment 7 yu shan wei 2018-08-03 03:20:41 UTC
(In reply to Ed Maste from comment #6)
I hope gpart can make a small partition table, 3 or 4....
Comment 8 yu shan wei 2018-08-07 02:24:49 UTC
sys/geom/part/g_part_gpt.c:
57:CTASSERT(sizeof(struct gpt_ent) == 128);
144:.gps_minent = 128,
470:if (hdr->hdr_entries == 0 || hdr->hdr_entsz < 128 ||
471:(hdr->hdr_entsz & 7) != 0)
是不是这几行限制了分区表的大小
Comment 9 Marcel Moolenaar freebsd_committer freebsd_triage 2019-03-05 04:05:00 UTC
At the very least mkimg(1) should round the number of entries to fill the sectors needed.
Comment 10 commit-hook freebsd_committer freebsd_triage 2019-03-05 04:15:45 UTC
A commit references this bug:

Author: marcel
Date: Tue Mar  5 04:15:34 UTC 2019
New revision: 344790
URL: https://svnweb.freebsd.org/changeset/base/344790

Log:
  Revert revision 254095

  In revision 254095, gpt_entries is not set to match the on-disk
  hdr_entries, but rather is computed based on available space.
  There are 2 problems with this:

  1.  The GPT backend respects hdr_entries and only reads and writes
      that number of partition entries.  On top of that, CRC32 is
      computed over the table that has hdr_entries elements.  When
      the common code works on what is possibly a larger number, the
      behaviour becomes inconsistent and problematic.  In particular,
      it would be possible to add a new partition that on a reboot
      isn't there anymore.
  2.  The calculation of gpt_entries is based on flawed assumptions.
      The GPT specification does not dictate that sectors are layed
      out in a particular way that the available space can be
      determined by looking at LBAs.  In practice, implementations
      do the same thing, because there's no reason to do it any
      other way.  Still, GPT allows certain freedoms that can be
      exploited in some form or shape if the need arises.

  PR:		229977
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D19438

Changes:
  head/sys/geom/part/g_part_gpt.c
Comment 11 Marcel Moolenaar freebsd_committer freebsd_triage 2019-03-05 04:18:14 UTC
Fixing in -CURRENT. Merges to release branches will happen after 2 weeks.
Comment 12 commit-hook freebsd_committer freebsd_triage 2019-03-22 23:39:52 UTC
A commit references this bug:

Author: marcel
Date: Fri Mar 22 23:39:16 UTC 2019
New revision: 345427
URL: https://svnweb.freebsd.org/changeset/base/345427

Log:
  MFC 344790:

  Revert revision 254095

  In revision 254095, gpt_entries is not set to match the on-disk
  hdr_entries, but rather is computed based on available space.
  There are 2 problems with this:

  1.  The GPT backend respects hdr_entries and only reads and writes
      that number of partition entries.  On top of that, CRC32 is
      computed over the table that has hdr_entries elements.  When
      the common code works on what is possibly a larger number, the
      behaviour becomes inconsistent and problematic.  In particular,
      it would be possible to add a new partition that on a reboot
      isn't there anymore.
  2.  The calculation of gpt_entries is based on flawed assumptions.
      The GPT specification does not dictate that sectors are layed
      out in a particular way that the available space can be
      determined by looking at LBAs.  In practice, implementations
      do the same thing, because there's no reason to do it any
      other way.  Still, GPT allows certain freedoms that can be
      exploited in some form or shape if the need arises.

  PR:		229977
  Differential Revision:	https://reviews.freebsd.org/D19438

Changes:
_U  stable/12/
  stable/12/sys/geom/part/g_part_gpt.c
Comment 13 commit-hook freebsd_committer freebsd_triage 2019-03-23 03:10:56 UTC
A commit references this bug:

Author: marcel
Date: Sat Mar 23 03:10:24 UTC 2019
New revision: 345434
URL: https://svnweb.freebsd.org/changeset/base/345434

Log:
  MFC 344790:

  Revert revision 254095

  In revision 254095, gpt_entries is not set to match the on-disk
  hdr_entries, but rather is computed based on available space.
  There are 2 problems with this:

  1.  The GPT backend respects hdr_entries and only reads and writes
      that number of partition entries.  On top of that, CRC32 is
      computed over the table that has hdr_entries elements.  When
      the common code works on what is possibly a larger number, the
      behaviour becomes inconsistent and problematic.  In particular,
      it would be possible to add a new partition that on a reboot
      isn't there anymore.
  2.  The calculation of gpt_entries is based on flawed assumptions.
      The GPT specification does not dictate that sectors are layed
      out in a particular way that the available space can be
      determined by looking at LBAs.  In practice, implementations
      do the same thing, because there's no reason to do it any
      other way.  Still, GPT allows certain freedoms that can be
      exploited in some form or shape if the need arises.

  PR:		229977
  Differential Revision:	https://reviews.freebsd.org/D19438

Changes:
_U  stable/11/
  stable/11/sys/geom/part/g_part_gpt.c