mdadm/Assemble: alloc superblock in Assemble #207
Conversation
|
@mwilck would you like to take a look? |
Assemble.c
Outdated
| * load_devices may release superblock which passes to it | ||
| * and alloc new superblock for it. | ||
| */ | ||
| *super = st; |
There was a problem hiding this comment.
Perhaps it would be more obvious if you just pass super instead of &st to load_devices?
There was a problem hiding this comment.
If you do that, use of st should probably be replaced by (*super).
There was a problem hiding this comment.
In this way, the patch will be a big one because st is used in many places after load_devices.
|
Hi all This patch introduces a regression problem which can be found by case 07autoassemble. The command is: After investigating, the related codes are: Now the logic depends on the original superblock every time. In this patch, it replaces the pointer if the original memory is released. So I plan to remove the first argument supertype in the Assemble function. I'll update this PR after fixing these problems. |
XiaoNi87
force-pushed
the
assemble-fix
branch
2 times, most recently
from
7af474e to
8a3a41d
Compare
XiaoNi87
changed the title
|
Hi all I updated the PR and regression test has passed: |
Now it allocs superblock outside Assemble and frees the memory outside Assemble. But the memory can be freed and realloc in Assemble. So freed memory will be dereferenced outside Assemble. This patch moves the memory management into Assemble. So it's more safe and the input arguments is less. This can be reproduced by: mdadm -CR /dev/md0 -l1 -n2 /dev/loop0 /dev/loop1 --assume-clean mdadm -Ss mdadm -A -e 1.2 /dev/md0 /dev/loop0 /dev/loop1 Signed-off-by: Xiao Ni <xni@redhat.com>
XiaoNi87
force-pushed
the
assemble-fix
branch
from
8a3a41d to
31c7341
Compare
|
Regression test pass with latest update: |
4 participants
load_devices may replaces superblock pointer if it finds the better one from member disks. In this way, it releases the memory which passes to Assemble by argument pointer st. The function main doesn't know this. So it may dereference a memory region which has been released after existing from Assemble.
This can be reproduced by:
mdadm -CR /dev/md0 -l1 -n2 /dev/loop0 /dev/loop1 --assume-clean mdadm -Ss
mdadm -A -e 1.2 /dev/md0 /dev/loop0 /dev/loop1
This patch uses a double pointer as the argument for Assemble. So it can reset the pointer after load_devices.