Commit Graph

2183 Commits

Author SHA1 Message Date
Mariusz Zaborski
31f7586d73 Introduce the 'n' flag for the geli attach command.
If the 'n' flag is provided the provided key number will be used to
decrypt device. This can be used combined with dryrun to verify if the key
is set correctly. This can be also used to determine which key slot we want to
change on already attached device.

Reviewed by:	allanjude
Differential Revision:	https://reviews.freebsd.org/D15309
2018-05-09 20:53:38 +00:00
Mark Johnston
bd92e6b6f5 Refactor some of the MI kernel dump code in preparation for netdump.
- Add clear_dumper() to complement set_dumper().
- Drain netdump's preallocated mbuf pool when clearing the dumper.
- Don't do bounds checking for dumpers with mediasize 0.
- Add dumper callbacks for initialization for writing out headers.

Reviewed by:	sbruno
MFC after:	1 month
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D15252
2018-05-06 00:22:38 +00:00
Mark Johnston
681554d70b Remove a redundant assertion.
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2018-05-06 00:05:03 +00:00
Mark Johnston
40e805221b Avoid dropping the topology lock in gmirror's dumpconf implementation.
Doing so introduces races which can lead to a use-after-free when
grabbing a snapshot of the GEOM mesh.

To ensure that a mirror's disk list remains stable, change its locking
protocol: both the softc lock and the topology lock are now required
to modify the list, so either lock is sufficient for traversal.

Tested by:	pho
MFC after:	2 weeks
Sponsored by:	Dell EMC Isilon
2018-05-06 00:03:24 +00:00
Ed Maste
b525a10ac0 gpart: add fat32lba MBR partition type
FAT32 partition with LBA addressing.

Reviewed by:	marcel
MFC after:	3 days
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D15266
2018-05-04 00:34:27 +00:00
Kyle Evans
74d6c131cb Annotate geom modules with MODULE_VERSION
GEOM ELI may double ask the password during boot. Once at loader time, and
once at init time.

This happens due a module loading bug. By default GEOM ELI caches the
password in the kernel, but without the MODULE_VERSION annotation, the
kernel loads over the kernel module, even if the GEOM ELI was compiled into
the kernel. In this case, the newly loaded module
purges/invalidates/overwrites the GEOM ELI's password cache, which causes
the double asking.

MFC Note: There's a pc98 component to the original submission that is
omitted here due to pc98 removal in head. This part will need to be revived
upon MFC.

Reviewed by:	imp
Submitted by:	op
Obtained from:	opBSD
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D14992
2018-04-10 19:18:16 +00:00
Mariusz Zaborski
8f1c45c20a Introduce dry run option for attaching the device.
This will allow us to verify if passphrase and key is valid without
decrypting whole device.

Reviewed by:	cem@, allanjude@
Differential Revision:	https://reviews.freebsd.org/D15000
2018-04-10 13:22:48 +00:00
Kyle Evans
2967ace894 Retire the geom_aes class
It's had a good life, but it's not really configurable and not really used.

Obtained from:	opBSD (with some changes)
Differential Revision:	https://reviews.freebsd.org/D14991
2018-04-09 17:30:30 +00:00
Brooks Davis
6469bdcdb6 Move most of the contents of opt_compat.h to opt_global.h.
opt_compat.h is mentioned in nearly 180 files. In-progress network
driver compabibility improvements may add over 100 more so this is
closer to "just about everywhere" than "only some files" per the
guidance in sys/conf/options.

Keep COMPAT_LINUX32 in opt_compat.h as it is confined to a subset of
sys/compat/linux/*.c.  A fake _COMPAT_LINUX option ensure opt_compat.h
is created on all architectures.

Move COMPAT_LINUXKPI to opt_dontuse.h as it is only used to control the
set of compiled files.

Reviewed by:	kib, cem, jhb, jtl
Sponsored by:	DARPA, AFRL
Differential Revision:	https://reviews.freebsd.org/D14941
2018-04-06 17:35:35 +00:00
Sean Bruno
2c385d51ce Squash error from geom by sizing ident strings to DISK_IDENT_SIZE.
Display attribute in future error strings and differentiate g_handleattr()
error messages for ease of debugging in the future.

"g_handleattr: md1 bio_length 24 strlen 31 -> EFAULT"

Reported by:	swills
Reviewed by:	imp cem avg
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D14962
2018-04-05 13:56:40 +00:00
Kirk McKusick
fb15890a8c When freeing a superblock returned by ffs_sbget, be sure to also
free the superblock summary information.

Reported by: Peter Holm (pho@)
Tested by: Peter Holm (pho@)
2018-03-24 15:36:25 +00:00
Mariusz Zaborski
9ea857cf0f Remove unneeded variable which was introduced in r328472.
Pointed out by:	pjd@
2018-03-18 15:09:55 +00:00
Andriy Gapon
aca41af247 g_access: deal with races created by geoms that drop the topology lock
The problem is that g_access() must be called with the GEOM topology
lock held.  And that gives a false impression that the lock is indeed
held across the call.  But this isn't always true because many classes,
ZVOL being one of the many, need to drop the lock.  It's either to
perform an I/O on the first open or to acquire a different lock (like in
g_mirror_access).

That, of course, can break many assumptions.  For example,
g_slice_access() adds an extra exclusive count on the first open. As
described above, an underlying geom may drop the topology lock and that
would open a race with another thread that would also request another
extra exclusive count.  In general, two consumers may be granted
incompatible accesses.

To avoid this problem the code is changed to mark a geom with special
flag before calling its access method and clear the flag afterwards.  If
another thread sees that flag, then it means that the topology lock has
been dropped (either by the geom in question or downstream from it), so
it is not safe to make another access call.  So, the second thread would
use g_topology_sleep() to wait until the flag is cleared and only then
would it proceed with the access.

Also see http://docs.freebsd.org/cgi/mid.cgi?809d9254-ee56-59d8-69a4-08838e985cea

PR:		225960
Reported by:	asomers
Reviewed by:	markj, mav
MFC after:	3 weeks
Differential Revision: https://reviews.freebsd.org/D14533
2018-03-15 09:16:10 +00:00
Conrad Meyer
ee4d316fe7 g_part_gpt: Fix memory leak in error path
If g_part_gpt_read() encountered a disk with bad primary and secondary
tables, it could leak memory.

Reported by:	Coverity
Sponsored by:	Dell EMC Isilon
2018-03-07 01:55:50 +00:00
Conrad Meyer
90575a0ec9 g_label_ufs: Fix typo from r330264
Reported by:	O. Hartmann <o.hartmann AT walstatt.org>
Sponsored by:	Dell EMC Isilon
2018-03-02 06:02:54 +00:00
Kirk McKusick
efbf396426 This change is some refactoring of Mark Johnston's changes in r329375
to fix the memory leak that I introduced in r328426. Instead of
trying to clear up the possible memory leak in all the clients, I
ensure that it gets cleaned up in the source (e.g., ffs_sbget ensures
that memory is always freed if it returns an error).

The original change in r328426 was a bit sparse in its description.
So I am expanding on its description here (thanks cem@ and rgrimes@
for your encouragement for my longer commit messages).

In preparation for adding check hashing to superblocks, r328426 is
a refactoring of the code to get the reading/writing of the superblock
into one place. Unlike the cylinder group reading/writing which
ends up in two places (ffs_getcg/ffs_geom_strategy in the kernel
and cgget/cgput in libufs), I have the core superblock functions
just in the kernel (ffs_sbfetch/ffs_sbput in ffs_subr.c which is
already imported into utilities like fsck_ffs as well as libufs to
implement sbget/sbput). The ffs_sbfetch and ffs_sbput functions
take a function pointer to do the actual I/O for which there are
four variants:

    ffs_use_bread / ffs_use_bwrite for the in-kernel filesystem

    g_use_g_read_data / g_use_g_write_data for kernel geom clients

    ufs_use_sa_read for the standalone code (stand/libsa/ufs.c
	but not stand/libsa/ufsread.c which is size constrained)

    use_pread / use_pwrite for libufs

Uses of these interfaces are in the UFS filesystem, geoms journal &
label, libsa changes, and libufs. They also permeate out into the
filesystem utilities fsck_ffs, newfs, growfs, clri, dump, quotacheck,
fsirand, fstyp, and quot. Some of these utilities should probably be
converted to directly use libufs (like dumpfs was for example), but
there does not seem to be much win in doing so.

Tested by: Peter Holm (pho@)
2018-03-02 04:34:53 +00:00
Mark Johnston
16759360d4 Fix a memory leak introduced in r328426.
ffs_sbget() may return a superblock buffer even if it fails, so the
caller must be prepared to free it in this case. Moreover, when tasting
alternate superblock locations in a loop, ffs_sbget()'s readfunc
callback must free the previously allocated buffer.

Reported and tested by:	pho
Reviewed by:		kib (previous version)
Differential Revision:	https://reviews.freebsd.org/D14390
2018-02-16 15:41:03 +00:00
Alan Somers
834063202a gpart: append partition name to the underlying provider's physical path
If the underlying provider's physical path is null, then the gpart device's
physical path will be, too. Otherwise, it will append the partition name,
such as "/p1" or "/s1/a". This will make gpart work better with zfsd(8).

PR:		224965
MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D14010
2018-02-14 20:26:09 +00:00
Alan Somers
0bab7fa8a7 geli: append "/eli" to the underlying provider's physical path
If the underlying provider's physical path is null, then the geli device's
physical path will be, too. Otherwise, it will append "/eli".  This will make
geli work better with zfsd(8).

PR:		224962
MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D13979
2018-02-14 20:15:32 +00:00
Justin Hibbits
d793587fe2 Fix a panic introduced in r329225
Some GEOM partition tables may be destroyed with incomplete partition
entries.  Guard against this with NULL checks.

Reported by:	pholm,others
Reviewed by:	markj
Tested by:	pholm
2018-02-14 15:12:09 +00:00
Justin Hibbits
08a3b42fdb Narrow a race, and fix a leak, in g_part_wither
A race in g_part_wither() can lead to I/O being performed with a freed GEOM
when the device disappears.  Close the race as best as we can for now,
following the code patterns from g_part_ctl_destroy() and g_part_ctl_undo().
This also fixes a leak, as g_wither_geom() does not wither providers, it
only orphans them, so the partition entries would never get destroyed in
g_wither_washer().

Note, this is not a complete fix, it can still race with g_part_start(), the
race has merely been narrowed.

Reviewed by:	markj
Sponsored by:	Dell EMC Isilon
2018-02-13 17:40:09 +00:00
Conrad Meyer
b42712a8b7 Add GUID and alias for Apple APFS partition
PR:		225813
Submitted by:	James Wright <james.wright AT jigsawdezign.com>
2018-02-11 06:57:20 +00:00
Mark Johnston
0d02f6c201 Simplify synchronization read error handling.
Since synchronization reads are performed by submitting a request to
the external mirror provider, we know that the request returns with an
error only when gmirror was unable to read a copy of the block from any
mirror. Thus, there is no need to retry the request from the
synchronization error handler.

Tested by:	pho
MFC after:	2 weeks
Sponsored by:	Dell EMC Isilon
2018-02-06 16:02:33 +00:00
Alan Somers
f5b4099e6b geom: don't write stack garbage in disk labels
Most consumers of g_metadata_store were passing in partially unallocated
memory, resulting in stack garbage being written to disk labels. Fix them by
zeroing the memory first.

gvirstor repeated the same mistake, but in the kernel.

Also, glabel's label contained a fixed-size string that wasn't
initialized to zero.

PR:		222077
Reported by:	Maxim Khitrov <max@mxcrypt.com>
Reviewed by:	cem
MFC after:	3 weeks
X-MFC-With:	323314
X-MFC-With:	323338
Differential Revision:	https://reviews.freebsd.org/D14164
2018-02-04 14:49:55 +00:00
Xin LI
90a48fba23 After r328426, g_label depends on UFS (option FFS) code to read UFS
superblock, and the kernel will fail to link when UFS is not built
in.  This commit makes it depend on a small portion of FFS bits and
thereby fixes build for this situation.

This is intended as an interim bandaid, and the actual superblock
reading code should probably be made independent of UFS, so we do
not need to depend on it (see kib@'s comment in the review for
details), and we will revisit this once the superblock check hashes
are all in place.

Differential Revision:	https://reviews.freebsd.org/D14092
2018-02-03 09:15:13 +00:00
Kirk McKusick
5d84ae8b49 Null out journal softc pointer earlier to avoid a segment fault
that can otherwise occur.

PR:           221804
Submitted by: Andreas Longwitz <longwitz at incore.de>
MFC after:    1 week
2018-01-31 23:30:49 +00:00
Mariusz Zaborski
0fc4adbe06 Don't truncate name of glabel.
If it's to long just report that.

Reviewed by:	trasz@
Differential Revision:	https://reviews.freebsd.org/D13746
2018-01-27 12:28:52 +00:00
Kirk McKusick
dffce2150e Refactoring of reading and writing of the UFS/FFS superblock.
Specifically reading is done if ffs_sbget() and writing is done
in ffs_sbput(). These functions are exported to libufs via the
sbget() and sbput() functions which then used in the various
filesystem utilities. This work is in preparation for adding
subperblock check hashes.

No functional change intended.

Reviewed by: kib
2018-01-26 00:58:32 +00:00
Pedro F. Giffuni
ac2fffa4b7 Revert r327828, r327949, r327953, r328016-r328026, r328041:
Uses of mallocarray(9).

The use of mallocarray(9) has rocketed the required swap to build FreeBSD.
This is likely caused by the allocation size attributes which put extra pressure
on the compiler.

Given that most of these checks are superfluous we have to choose better
where to use mallocarray(9). We still have more uses of mallocarray(9) but
hopefully this is enough to bring swap usage to a reasonable level.

Reported by:	wosch
PR:		225197
2018-01-21 15:42:36 +00:00
Alan Somers
6f7f85e0e1 gnop(8): add the ability to set a nop provider's physical path
While I'm here, expand the existing tests a bit.

MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D13579
2018-01-18 05:57:10 +00:00
Pedro F. Giffuni
0cee6dcdb0 misc geom and gnu: make some use of mallocarray(9).
Focus on code where we are doing multiplications within malloc(9). None of
these ire likely to overflow, however the change is still useful as some
static checkers can benefit from the allocation attributes we use for
mallocarray.

This initial sweep only covers malloc(9) calls with M_NOWAIT. No good
reason but I started doing the changes before r327796 and at that time it
was convenient to make sure the sorrounding code could handle NULL values.

Differential revision: https://reviews.freebsd.org/D13837
2018-01-15 21:23:16 +00:00
Andriy Gapon
6ce374aa94 geom_disk / scsi_da: deny opening write-protected disks for writing
Ths change consists of two parts.

geom_disk: deny opening a disk for writing if it's marked as
write-protected.  A new disk(9) flag is added to mark write protected
disks.  A possible alternative could be to add another parameter to d_open,
so that the open mode could be passed to it and the disk drivers could
make the decision internally, but the flag required less churn.

scsi_da: add a new phase of disk probing to query the all pages mode
sense page.  We can determine if the disk is write protected using bit 7
of the device specific field in the mode parameter header returned by
MODE SENSE.

PR:		224037
Reviewed by:	mav
MFC after:	4 weeks
Differential Revision: https://reviews.freebsd.org/D13360
2018-01-15 11:20:00 +00:00
Mark Johnston
762f440f15 Fix handling of read errors during mirror synchronization.
We would previously just free the request BIO, which would either cause
the disk to stay stuck in the SYNCHRONIZING state, or result in
synchronization completing without having copied the block which
returned an error.

With this change, if the disk which returned an error is the only active
disk in the mirror, the synchronizing disk is kicked out. Otherwise, the
read is retried.

Reported and tested by:	pho (previous version)
MFC after:	2 weeks
Sponsored by:	Dell EMC Isilon
2018-01-10 19:37:21 +00:00
Mark Johnston
792f0c3b09 Clarify the use of the gmirror flag mask constants.
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2018-01-10 15:21:36 +00:00
Mark Johnston
aed882a9fb Avoid referencing a possibly freed consumer after r327496.
g_mirror_regular_request() may free the gmirror consumer for a disk
if that disk is being disconnected, after which we must not dereference
the consumer pointer.

CID:		1384280
X-MFC with:	r327496
2018-01-10 05:06:21 +00:00
Mark Johnston
8b0a00b745 Sort and remove unneeded includes.
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2018-01-08 15:56:40 +00:00
Mark Johnston
7653e6d781 Release the queue lock before restarting the worker loop.
Reported and tested by:	pho
MFC after:	3 days
Sponsored by:	Dell EMC Isilon
2018-01-08 15:41:49 +00:00
Mark Johnston
1787c3feb4 Fix some I/O ordering issues in gmirror.
- BIO_FLUSH requests were dispatched to the disks directly from
  g_mirror_start() rather than going through the mirror's I/O request
  queue, so they could have been reordered with preceding writes.
  Address this by processing such requests from the queue, avoiding
  direct dispatch.
- Handling for collisions with synchronization requests was too
  fine-grained and could cause reordering of writes. In particular,
  BIO_ORDERED was not being honoured. Address this by effectively
  freezing the request queue any time a collision with a synchronization
  request occurs. The queue is unfrozen once the collision with the
  first frozen request is over.
- The above-mentioned collision handling allowed reads to jump ahead
  of writes to the same offset. Address this by freezing all request
  types when a collision occurs, not just BIO_WRITEs and BIO_DELETEs.

Also add some more fail points for use in testing error handling.

Reviewed by:	imp
MFC after:	3 weeks
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D13559
2018-01-02 18:11:54 +00:00
Colin Percival
8b8a7c43a9 Instrument "boot holds" for the benefit of the TSLOG framework. These
are places where the "main thread" of the booting kernel (either the
thread which later becomes swapper or the thread which later becomes
init) has to stop and wait for action to take place in another thread
before continuing.

There are currently three such holds:
1. The intr_config_hooks SYSINIT waits for hooks registered via the
config_intrhook_establish function; this allows (typically) devices
which need interrupts enabled to complete their initialization to do
so before root is mounted.

2. The g_waitidle function waits for the GEOM event queue to be empty;
this ensures that all of the disks which have been attached have been
tasted before we attempt to mount root.

3. The vfs_mountroot_wait function (in addition to calling g_waitidle)
waits for holds registered via root_mount_hold; among other things, this
is used by the USB subsystem to ensure that we don't fail to mount root
if it's located on a USB disk which takes a while to probe.
2017-12-31 09:23:52 +00:00
Pedro F. Giffuni
2afb21f309 geom_ccd.c: Fix the licenses properly
The license merging in r109471 didn't take into account that licensing
could change. Just removing the 3rd clause obviates the copyright
assignment to the NetBSD Foundation.

We do have plenty of files that have two or more licensing as in this
case, so fix this properly by splitting back the licenses as they are
upstream.

Obtained from:	NetBSD
2017-12-30 02:07:18 +00:00
Pedro F. Giffuni
68689f580b geom_ccd.c: Update the license with changes from upstream.
Part of this file originated in NetBSD, with the original file
carrying two versions of 4-clause BSD licenses. r109471 attempted to
simplify the situation by putting both licenses together.

Meanwhile, NetBSD dropped Clauses 3 and 4 from their own license, and
eventually NetBSD got permission from the University of Utah to drop the
3rd clause.

Keep the license "simple" by dropping the third clause since both TNF,
Utah/Berkeley and phk agree in principle that it can be dropped.

Obtained from:	NetBSD (ccd.c CVS 1.128, 1.138)
2017-12-30 01:37:08 +00:00
Alexander Kabaev
151ba7933a Do pass removing some write-only variables from the kernel.
This reduces noise when kernel is compiled by newer GCC versions,
such as one used by external toolchain ports.

Reviewed by: kib, andrew(sys/arm and sys/arm64), emaste(partial), erj(partial)
Reviewed by: jhb (sys/dev/pci/* sys/kern/vfs_aio.c and sys/kern/kern_synch.c)
Differential Revision: https://reviews.freebsd.org/D10385
2017-12-25 04:48:39 +00:00
Mark Johnston
9abe2e7e98 Avoid using bioq_* in gmirror.
gmirror does not perform any sorting of I/O requests, so the bioq API
doesn't provide any advantages over plain TAILQs. The API also does not
provide operations needed by an upcoming change.

No functional change intended. The diff shrinks the geom_mirror.ko
text and the gmirror softc slightly.

Tested by:	pho (part of a larger patch)
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-19 17:13:04 +00:00
Mark Johnston
68eadcec0f Give a couple of predication functions a bool return type.
No functional change intended.

MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-15 19:14:21 +00:00
Mark Johnston
204d94f161 Typo.
MFC after:	1 week
2017-12-15 19:03:03 +00:00
Mark Johnston
8b93770503 Address a possible lost wakeup for gmirror events.
g_mirror_event_send() acquires the I/O queue lock to deliver a wakeup
to the worker thread, and this is done after enqueuing the event.
So it's sufficient to check the event queue before atomically releasing
the queue lock and going to sleep.

MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-12 17:29:34 +00:00
Mark Johnston
b634781eac Give g_mirror_event_get() a more accurate name.
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-12 17:25:25 +00:00
Mark Johnston
a3584ee355 Decrement sc_writes when BIO_DELETE requests complete.
Otherwise a gmirror that has received a BIO_DELETE request will never be
marked clean (unless sc_writes overflows).

MFC after:	1 week
Sponsored by:	Dell EMC Isilon
2017-12-12 17:24:30 +00:00
Eugene Grosbein
1b8ea9beff geom_raid (RAID5): do not lose bp->bio_error, keep it in pbp->bio_error
and return it by passing to g_raid_iodone()

Approved by:	mav (mentor)
MFC after:	3 days
2017-12-07 20:09:17 +00:00
Eugene Grosbein
01a51aba38 Fix use-after-free that sometimes results in a garbage returned
instead of right error code after requests to SINGLE/CONCAT volumes, f.e:

# dd if=/dev/raid/r0 bs=512 of=/dev/null
dd: /dev/raid/r0: Unknown error: -559038242

Reviewed by:	avg (mentor), mav (mentor)
MFC after:	3 days
2017-12-07 05:55:18 +00:00