In util.c, there is a building error:
'/md/metadata_version' directive writing 20 bytes into a
region of size between 0 and 255 [-Werror=format-overflow=]
In mapfile.c
It declares the fouth argument as 'int *' in map_update,
but in mdadm.h it's previously declared as an array 'int[4]'
Signed-off-by: Xiao Ni <xni@redhat.com>
Signed-off-by: Jes Sorensen <jsorensen@fb.com>
We can see "double free or corruption" with below steps
as reported by Mariusz:
export IMSM_NO_PLATFORM=1
export IMSM_DEVNAME_AS_SERIAL=1
mdadm --zero-super /dev/sd*
mdadm -C /dev/md/imsm -n2 -eimsm /dev/sdb /dev/sdc --run
mdadm -C /dev/md/r1 -n2 -z15G -eimsm /dev/sdb /dev/sdc -l1 --run --assume-clean
mdadm -f /dev/md126 /dev/sdb
mdadm -Ss
It is caused by Manage_stop calls map_remove and map_unlock,
but *mapp is not set to NULL after map_remove -> map_free,
so map_unlock will call map_free again.
Reported-by: Tkaczyk Mariusz <mariusz.tkaczyk@intel.com>
Tested-by: Tkaczyk Mariusz <mariusz.tkaczyk@intel.com>
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
Signed-off-by: Jes Sorensen <jsorensen@fb.com>
1. There are some places which didn't free map as
discovered by coverity.
CID 289661 (#1 of 1): Resource leak (RESOURCE_LEAK)12. leaked_storage: Variable mapl going out of scope leaks the storage it points to.
CID 289619 (#3 of 3): Resource leak (RESOURCE_LEAK)63. leaked_storage: Variable map going out of scope leaks the storage it points to.
CID 289618 (#1 of 1): Resource leak (RESOURCE_LEAK)26. leaked_storage: Variable map going out of scope leaks the storage it points to.
CID 289607 (#1 of 1): Resource leak (RESOURCE_LEAK)41. leaked_storage: Variable map going out of scope leaks the storage it points to.
2. If we call map_by_* inside a loop, then map_free
should be called in the same loop, and it is better
to set map to NULL after free.
3. And map_unlock is always called with map_lock,
if we don't call map_remove before map_unlock,
then the memory (allocated by map_lock -> map_read
-> map_add -> xmalloc) could be leaked. So we
need to free it in map_unlock as well.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
Signed-off-by: Jes Sorensen <jsorensen@fb.com>
Commit 4dd2df0966 added a trip through makedev(), major(), and minor() for
device major and minor numbers. This would cause mdadm to fail in operating
on a device with a minor number bigger than (2^19)-1 due to it changing
from dev_t to a signed int and back.
Where this was found as a problem was when a array was created with a device
specified as a name like /dev/md/raidname and there were already 128 arrays
on the system. In this case, mdadm would chose 1048575 ((2^20)-1) for the
array and minor number. This would cause the major and minor number to become
negative when generated from devnm2devid() and passed to major() and minor()
in open_dev_excl(). open_dev_excl() would then call dev_open() which would
detect the negative minor number and call open() on the *char containing the
major:minor pair which isn't a valid file.
Signed-off-by: Mike Lovell <mlovell@bluehost.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
When /run/mdadm/map is being rebuilt, e.g. by "mdadm -Ir",
if the device doesn't exist in /dev, we have to choose
a name.
Currently we don't strip the hostname which is wrong if
it is the local host.
Reported-by: Stephen Kent <smkent@smkent.net>
Signed-off-by: NeilBrown <neilb@suse.de>
We widely use a "devnum" which is 0 or +ve for md%d devices
and -ve for md_d%d devices.
But I want to be able to use md_%s device names.
So get rid of devnum (a number) and use devnm (a 32char string).
eg.
md0
md_d2
md_home
Signed-off-by: NeilBrown <neilb@suse.de>
1/ When printing the "name=" entry for --brief output,
enclose name in quotes if it contains spaces etc.
Quotes are already supported for reading mdadm.conf
2/ When a name is used as a device name, translate spaces
and tabs to '_', as well as the current translation of
'/' to '-'.
Signed-off-by: NeilBrown <neilb@suse.de>
When recreating the mapfile entry for a container we need to
use ->getinfo_super, not ->container_content, just like we
do in Detail().
Signed-off-by: NeilBrown <neilb@suse.de>
malloc should never fail, and if it does it is unlikely
that anything else useful can be done. Best approach is to
abort and let some super-daemon restart.
So define xmalloc, xcalloc, xrealloc, xstrdup which don't
fail but just print a message and exit. Then use those
removing all the tests for failure.
Also replace all "malloc;memset" sequences with 'xcalloc'.
Signed-off-by: NeilBrown <neilb@suse.de>
Now that /run seems to be a good standard, make that
the default for storing various run-time files, rather than
/var/run or /dev/.mdadm.
Signed-off-by: NeilBrown <neilb@suse.de>
Map file may miss an entry if bad flag is not cleared on update.
This happens for example when an old entry exists in map that
has no mdstat counterpart and we create a new array with the same devnum.
Newly created array will not appear in map if update doesnt clear bad flag.
Signed-off-by: Anna Czarnowska <anna.czarnowska@intel.com>
Signed-off-by: NeilBrown <neilb@suse.de>
search_mdstat and conf_match are almost identical.
Put all the functionality in conf_match, and remove search_mdstat.
Reported-by: Jes.Sorensen@redhat.com
Signed-off-by: NeilBrown <neilb@suse.de>
1. Three missing map_unlock() calls were added.
2. Map file must be unlocked on fork, else child will hold lock.
Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
Signed-off-by: NeilBrown <neilb@suse.de>
Now that we can ask container_content for a specific subarray,
we don't need to pass the subarray name to load_super, and have it
secretly modify the returned state.
Signed-off-by: NeilBrown <neilb@suse.de>
We don't need ->container_dev here, and we will soon be passing
subarray as an explicit arg to load_super.
So simplify extraction of subarray and move the strcpy close to
->load_super.
Signed-off-by: NeilBrown <neilb@suse.de>
To accurately detect when an array has been split and is now being
recombined, we need to track which other devices each thinks is
working.
We should never include a device in an array if it thinks that the
primary device has failed.
This patch just allows get_info_super to return a list of devices
and whether they are thought to be working or not.
Signed-off-by: NeilBrown <neilb@suse.de>
Having multiple possible locations and guessing where best to put the
file is too messy, confusing and makes locking problematic.
So just keep it in /dev/.mdadm/map. It is a horrible place but it is
really all we have. System integrators can change this easily at
build time.
Signed-off-by: NeilBrown <neilb@suse.de>
- Update the comments
- use some defined names instead of magic numbers.
- restore /var/run/mdadm/map to have priority over /dev/.mdadm/map
Signed-off-by: NeilBrown <neilb@suse.de>
While we attempt to use a lockfile to grant exclusive access to the
mapfile, our implementation is buggy. Specifically, we create a lockfile,
then lock it, at which point new instances can open the lockfile and
attempt to lock it, which will cause them to block. However, when we are
ready to unlock it, we unlink the file. This causes existing lock waiters
to get a lock on an unlinked inode while a different instance may now
create a new lockfile and get an exclusive lock on it.
There are several possible fixes. The chosen one is to test if
->s_nlink is zero after we get the lock and to retry if it isn't.
This means:
- failing to unlink a file doesn't leave a stale lock
- we can block waiting to get a lock rather than busy-waiting
- we don't need to leave a lock file permanently in place.
Reported-by: Doug Ledford <dledford@redhat.com>
Signed-off-by: NeilBrown <neilb@suse.de>
We now have 3 directory definitions: mdmon directory for its pid and
sock files (compile time define, not changable at run time), mdmonitor
directory which is for the mdadm monitor mode pid file (can only be
passed in via command line at the time mdadm is invoked in monitor mode),
and the directory for the mdadm incremental assembly map file (compile
time define, not changable at run time). Only the mdadm map file still
hunts multiple locations, and the number of locations has been reduced
to /var/run and the compile time specified location. Re-use of similar
sounding defines that actually didn't denote their actual usage at
compile time made it more difficult for a person to know what affect
changing the compile time defines would have on the resulting programs.
This patch renames the various defines to clearly identify which item
the define affects. It also reduces the number of various directories
which will be searched for these files as this has lead to confusion
in mdadm and mdmon in terms of which files should take precedence when
files exist in multiple locations, etc. It's best if the person
compiling the program intentionally and with planning selects the
right directories to be used for the various purposes. Which directory
is right depends on which items you are talking about and what boot
loader your system uses and what initramfs generation program your
system uses. Because of the inter-dependency of all these items it
would typically be up to the distribution that mdadm is being integrated
into to select the correct values for these defines.
Signed-off-by: Doug Ledford <dledford@redhat.com>
...i.e. GET_DEVS == (GET_DEVS|SKIP_GONE_DEVS)
A null pointer dereference in Incremental.c can be triggered by
replugging a disk while the old name is in use. When mdadm -I is called
on the new disk we fail the call to sysfs_read(). I audited all the
locations that use GET_DEVS and it appears they can tolerate missing a
drive. So just make SKIP_GONE_DEVS the default behaviour.
Also fix up remaining unchecked usages of the sysfs_read() return value.
Reported-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
mdadm prevents creation when device names are duplicated on the command
line, but leaves the partially created array intact. Detect this case
in the error code from add_to_super() and cleanup the partially created
array. The imsm handler is updated to report this conflict in
add_to_super_imsm_volume().
Note that since neither mdmon, nor userspace for that matter, ever saw an
active array we only need to perform a subset of the cleanup actions.
So call ioctl(STOP_ARRAY) directly and arrange for Create() to cleanup
the map file rather than calling Manage_runstop().
Reported-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
filesystem is writable).
This particularly keeps udev happy if VAR_RUN is set to /dev/md.
Signed-off-by: Doug Ledford <dledford@redhat.com>
Signed-off-by: NeilBrown <neilb@suse.de>
The current locking uses lockf, and is completely broken.
When you hold a lockf lock, any close of any fd on that file will
release the lock. So map_read() call which is made as soon as we get
the lock, will immediately drop the lock.
So change to flock locking which isn't so badly designed.
Signed-off-by: NeilBrown <neilb@suse.de>
RebuildMap falsely returns container info for member arrays. Retrieving
the subarray and container_dev details prior to ->load_super() changes the
result from:
md127 imsm 082c6371:74b5ce03:64972e41:6b0860d5 /dev/md/imsm
md126 imsm 082c6371:74b5ce03:64972e41:6b0860d5 /dev/md/vol0
...to:
md126 /md127/0 3e03aee2:78c3c593:1e8ecaf0:eefb53ed /dev/md/vol0
md127 imsm 082c6371:74b5ce03:64972e41:6b0860d5 /dev/md/imsm
Reported-by: Ignacy Kasperowicz <ignacy.kasperowicz@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
When rebuilding the mapfile (mdadm -Ir), if not appropriate name is
found in /dev/md/, try to find an appropriate name, either by looking
in mdadm.conf or by using the name in the metadata.
Signed-off-by: NeilBrown <neilb@suse.de>
During early boot, /var/run may not exist or be writable.
If that happens, sore the mapfile (which is very important for
incremental assembly) in /dev (which should exist for udev).
Thanks to Doug Ledford <dledford@redhat.com> for identify this
problem and suggesting a solution.
Signed-off-by: NeilBrown <neilb@suse.de>
When rebuilding the map file tolerate missing/offline disks, otherwise
we will segfault on the NULL return from sysfs_read.
Reported-by: Jacek Danecki <jacek.danecki@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
It is possible for some arrays to be created e.g. by initrd, and so
not get mentioned in /var/run/mdadm/map.
As "-I" depends on things being listed in 'map', we create it by
scanning all devices if it doesn't exist.
Signed-off-by: NeilBrown <neilb@suse.de>
It is possible for the mapfile to become wrong, and that gets
very confusing. So validate entries before returning them.
Signed-off-by: NeilBrown <neilb@suse.de>
Rather than appending the md minor number, we now append a small
sequence number to make sure name in /dev/md/ that aren't LOCAL are
unique. As the map file is locked while we do this, we are sure
of no losing any races.
Signed-off-by: NeilBrown <neilb@suse.de>
This only applies if udev isn't installed or is disabled
by MDADM_NO_UDEV
We try to remove partitions too.
We find names to remove by looking in /var/run/mdadm/map
Signed-off-by: NeilBrown <neilb@suse.de>
We previously only updated /var/run/mdadm/map when starting an
array with --incremental. However we now make more use of
that file (to pass the dev name to udev) so always update it.
Signed-off-by: NeilBrown <neilb@suse.de>
Xiao Ni
Guoqing Jiang
Jes Sorensen
Mike Lovell
Bas van Schaik
NeilBrown
Czarnowska, Anna
Lukasz Dorau
Doug Ledford
Dan Williams