Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1600281 > unrolled thread
| Started by | Michael Ellerman <mpe@ellerman.id.au> |
|---|---|
| First post | 2017-03-14 13:20 +0100 |
| Last post | 2017-03-16 19:10 +0100 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t Michael Ellerman <mpe@ellerman.id.au> - 2017-03-14 13:20 +0100
RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-14 13:40 +0100
Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-03-14 16:00 +0100
RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-16 19:10 +0100
| From | Michael Ellerman <mpe@ellerman.id.au> |
|---|---|
| Date | 2017-03-14 13:20 +0100 |
| Subject | Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t |
| Message-ID | <tkTAK-8qq-23@gated-at.bofh.it> |
Elena Reshetova <elena.reshetova@intel.com> writes: > refcount_t type and corresponding API should be > used instead of atomic_t when the variable is used as > a reference counter. This allows to avoid accidental > refcounter overflows that might lead to use-after-free > situations. > > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > Signed-off-by: David Windsor <dwindsor@gmail.com> > --- > drivers/md/md.c | 6 +++--- > drivers/md/md.h | 3 ++- > 2 files changed, 5 insertions(+), 4 deletions(-) When booting linux-next (specifically 5be4921c9958ec) I'm seeing the backtrace below. I suspect this patch is just exposing an existing issue? cheers [ 0.230738] md: Waiting for all devices to be available before autodetect [ 0.230742] md: If you don't use raid, use raid=noautodetect [ 0.230962] refcount_t: increment on 0; use-after-free. [ 0.230988] ------------[ cut here ]------------ [ 0.230996] WARNING: CPU: 0 PID: 1 at lib/refcount.c:114 .refcount_inc+0x5c/0x70 [ 0.231001] Modules linked in: [ 0.231006] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc1-gccN-next-20170310-g5be4921 #1 [ 0.231012] task: c000000049400000 task.stack: c000000049440000 [ 0.231016] NIP: c0000000005ac6bc LR: c0000000005ac6b8 CTR: c000000000743390 [ 0.231021] REGS: c000000049443160 TRAP: 0700 Not tainted (4.11.0-rc1-gccN-next-20170310-g5be4921) [ 0.231026] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> [ 0.231033] CR: 24024422 XER: 0000000c [ 0.231038] CFAR: c000000000a5356c SOFTE: 1 [ 0.231038] GPR00: c0000000005ac6b8 c0000000494433e0 c000000001079d00 000000000000002b [ 0.231038] GPR04: 0000000000000000 00000000000000ef 0000000000000000 c0000000010418a0 [ 0.231038] GPR08: 000000004af80000 c000000000ecc9a8 c000000000ecc9a8 0000000000000000 [ 0.231038] GPR12: 0000000028024824 c000000006bb0000 0000000000000000 c000000049443a00 [ 0.231038] GPR16: 0000000000000000 c000000049443a10 0000000000000000 0000000000000000 [ 0.231038] GPR20: 0000000000000000 0000000000000000 c000000000f7dd20 0000000000000000 [ 0.231038] GPR24: 00000000014080c0 c0000000012060b8 c000000001206080 0000000000000009 [ 0.231038] GPR28: c000000000f7dde0 0000000000900000 0000000000000000 c0000000461ae800 [ 0.231100] NIP [c0000000005ac6bc] .refcount_inc+0x5c/0x70 [ 0.231104] LR [c0000000005ac6b8] .refcount_inc+0x58/0x70 [ 0.231108] Call Trace: [ 0.231112] [c0000000494433e0] [c0000000005ac6b8] .refcount_inc+0x58/0x70 (unreliable) [ 0.231120] [c000000049443450] [c00000000086c008] .mddev_find+0x1e8/0x430 [ 0.231125] [c000000049443530] [c000000000872b6c] .md_open+0x2c/0x140 [ 0.231132] [c0000000494435c0] [c0000000003962a4] .__blkdev_get+0xd4/0x520 [ 0.231138] [c000000049443690] [c000000000396cc0] .blkdev_get+0x1c0/0x4f0 [ 0.231145] [c000000049443790] [c000000000336d64] .do_dentry_open.isra.1+0x2a4/0x410 [ 0.231152] [c000000049443830] [c0000000003523f4] .path_openat+0x624/0x1580 [ 0.231157] [c000000049443990] [c000000000354ce4] .do_filp_open+0x84/0x120 [ 0.231163] [c000000049443b10] [c000000000338d74] .do_sys_open+0x214/0x300 [ 0.231170] [c000000049443be0] [c000000000da69ac] .md_run_setup+0xa0/0xec [ 0.231176] [c000000049443c60] [c000000000da4fbc] .prepare_namespace+0x60/0x240 [ 0.231182] [c000000049443ce0] [c000000000da47a8] .kernel_init_freeable+0x330/0x36c [ 0.231190] [c000000049443db0] [c00000000000dc44] .kernel_init+0x24/0x160 [ 0.231197] [c000000049443e30] [c00000000000badc] .ret_from_kernel_thread+0x58/0x7c [ 0.231202] Instruction dump: [ 0.231206] 60000000 3d22ffee 89296bfb 2f890000 409effdc 3c62ffc6 39200001 3d42ffee [ 0.231216] 38630928 992a6bfb 484a6e79 60000000 <0fe00000> 4bffffb8 60000000 60000000 [ 0.231226] ---[ end trace 8c51f269ad91ffc2 ]--- [ 0.231233] md: Autodetecting RAID arrays. [ 0.231236] md: autorun ... [ 0.231239] md: ... autorun DONE. [ 0.234188] EXT4-fs (sda4): mounting ext3 file system using the ext4 subsystem [ 0.250506] refcount_t: underflow; use-after-free. [ 0.250531] ------------[ cut here ]------------ [ 0.250537] WARNING: CPU: 0 PID: 3 at lib/refcount.c:207 .refcount_dec_not_one+0x104/0x120 [ 0.250542] Modules linked in: [ 0.250546] CPU: 0 PID: 3 Comm: kworker/0:0 Tainted: G W 4.11.0-rc1-gccN-next-20170310-g5be4921 #1 [ 0.250553] Workqueue: events .delayed_fput [ 0.250557] task: c000000049404900 task.stack: c000000049448000 [ 0.250562] NIP: c0000000005ac964 LR: c0000000005ac960 CTR: c000000000743390 [ 0.250567] REGS: c00000004944b530 TRAP: 0700 Tainted: G W (4.11.0-rc1-gccN-next-20170310-g5be4921) [ 0.250572] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> [ 0.250578] CR: 24002422 XER: 00000007 [ 0.250584] CFAR: c000000000a5356c SOFTE: 1 [ 0.250584] GPR00: c0000000005ac960 c00000004944b7b0 c000000001079d00 0000000000000026 [ 0.250584] GPR04: 0000000000000000 0000000000000113 0000000000000000 c0000000010418a0 [ 0.250584] GPR08: 000000004af80000 c000000000ecc9a8 c000000000ecc9a8 0000000000000000 [ 0.250584] GPR12: 0000000022002824 c000000006bb0000 c0000000001116d0 c000000049050200 [ 0.250584] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 [ 0.250584] GPR20: 0000000000000001 0000000000000000 c000000048030a98 0000000000000001 [ 0.250584] GPR24: 000000000002001d 0000000000000000 0000000000000000 c0000000461af000 [ 0.250584] GPR28: 0000000000000000 c000000048030bd8 c0000000461aea08 c0000000012060b8 [ 0.250645] NIP [c0000000005ac964] .refcount_dec_not_one+0x104/0x120 [ 0.250650] LR [c0000000005ac960] .refcount_dec_not_one+0x100/0x120 [ 0.250654] Call Trace: [ 0.250658] [c00000004944b7b0] [c0000000005ac960] .refcount_dec_not_one+0x100/0x120 (unreliable) [ 0.250665] [c00000004944b820] [c0000000005ac9a0] .refcount_dec_and_lock+0x20/0xc0 [ 0.250671] [c00000004944b8a0] [c000000000870fa4] .mddev_put+0x34/0x180 [ 0.250677] [c00000004944b930] [c000000000396108] .__blkdev_put+0x288/0x350 [ 0.250683] [c00000004944ba30] [c0000000003968f0] .blkdev_close+0x30/0x50 [ 0.250689] [c00000004944bab0] [c00000000033e7d8] .__fput+0xc8/0x2a0 [ 0.250695] [c00000004944bb60] [c00000000033ea08] .delayed_fput+0x58/0x80 [ 0.250701] [c00000004944bbe0] [c000000000107ea0] .process_one_work+0x2a0/0x630 [ 0.250707] [c00000004944bc80] [c0000000001082c8] .worker_thread+0x98/0x6a0 [ 0.250713] [c00000004944bd70] [c000000000111868] .kthread+0x198/0x1a0 [ 0.250719] [c00000004944be30] [c00000000000badc] .ret_from_kernel_thread+0x58/0x7c [ 0.250724] Instruction dump: [ 0.250728] 419e000c 38210070 4e800020 7c0802a6 3c62ffc6 39200001 3d42ffee 38630958 [ 0.250738] 992a6bfe f8010080 484a6bd1 60000000 <0fe00000> e8010080 38600001 7c0803a6 [ 0.250748] ---[ end trace 8c51f269ad91ffc3 ]--- [ 0.262454] EXT4-fs (sda4): mounted filesystem with ordered data mode. Opts: (null)
[toc] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2017-03-14 13:40 +0100 |
| Subject | RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t |
| Message-ID | <tkTU6-5w-37@gated-at.bofh.it> |
| In reply to | #1600281 |
> Elena Reshetova <elena.reshetova@intel.com> writes: > > > refcount_t type and corresponding API should be > > used instead of atomic_t when the variable is used as > > a reference counter. This allows to avoid accidental > > refcounter overflows that might lead to use-after-free > > situations. > > > > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > > Signed-off-by: Kees Cook <keescook@chromium.org> > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > --- > > drivers/md/md.c | 6 +++--- > > drivers/md/md.h | 3 ++- > > 2 files changed, 5 insertions(+), 4 deletions(-) > > When booting linux-next (specifically 5be4921c9958ec) I'm seeing the > backtrace below. I suspect this patch is just exposing an existing > issue? Yes, we have actually been following this issue in the another thread. It looks like the object is re-used somehow, but I can't quite understand how just by reading the code. This was what I put into the previous thread: "The log below indicates that you are using your refcounter in a bit weird way in mddev_find(). However, I can't find the place (just by reading the code) where you would increment refcounter from zero (vs. setting it to one). It looks like you either iterate over existing nodes (and increment their counters, which should be >= 1 at the time of increment) or create a new node, but then mddev_init() sets the counter to 1. " If you can help to understand what is going on with the object creation/destruction, would be appreciated! Also Shaohua Li stopped this patch coming from his tree since the issue was caught at that time, so we are not going to merge this until we figure it out. Best Regards, Elena. > > cheers > > > [ 0.230738] md: Waiting for all devices to be available before autodetect > [ 0.230742] md: If you don't use raid, use raid=noautodetect > [ 0.230962] refcount_t: increment on 0; use-after-free. > [ 0.230988] ------------[ cut here ]------------ > [ 0.230996] WARNING: CPU: 0 PID: 1 at lib/refcount.c:114 > .refcount_inc+0x5c/0x70 > [ 0.231001] Modules linked in: > [ 0.231006] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc1-gccN-next- > 20170310-g5be4921 #1 > [ 0.231012] task: c000000049400000 task.stack: c000000049440000 > [ 0.231016] NIP: c0000000005ac6bc LR: c0000000005ac6b8 CTR: > c000000000743390 > [ 0.231021] REGS: c000000049443160 TRAP: 0700 Not tainted (4.11.0-rc1- > gccN-next-20170310-g5be4921) > [ 0.231026] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> > [ 0.231033] CR: 24024422 XER: 0000000c > [ 0.231038] CFAR: c000000000a5356c SOFTE: 1 > [ 0.231038] GPR00: c0000000005ac6b8 c0000000494433e0 c000000001079d00 > 000000000000002b > [ 0.231038] GPR04: 0000000000000000 00000000000000ef 0000000000000000 > c0000000010418a0 > [ 0.231038] GPR08: 000000004af80000 c000000000ecc9a8 c000000000ecc9a8 > 0000000000000000 > [ 0.231038] GPR12: 0000000028024824 c000000006bb0000 0000000000000000 > c000000049443a00 > [ 0.231038] GPR16: 0000000000000000 c000000049443a10 0000000000000000 > 0000000000000000 > [ 0.231038] GPR20: 0000000000000000 0000000000000000 c000000000f7dd20 > 0000000000000000 > [ 0.231038] GPR24: 00000000014080c0 c0000000012060b8 c000000001206080 > 0000000000000009 > [ 0.231038] GPR28: c000000000f7dde0 0000000000900000 0000000000000000 > c0000000461ae800 > [ 0.231100] NIP [c0000000005ac6bc] .refcount_inc+0x5c/0x70 > [ 0.231104] LR [c0000000005ac6b8] .refcount_inc+0x58/0x70 > [ 0.231108] Call Trace: > [ 0.231112] [c0000000494433e0] [c0000000005ac6b8] .refcount_inc+0x58/0x70 > (unreliable) > [ 0.231120] [c000000049443450] [c00000000086c008] > .mddev_find+0x1e8/0x430 > [ 0.231125] [c000000049443530] [c000000000872b6c] .md_open+0x2c/0x140 > [ 0.231132] [c0000000494435c0] [c0000000003962a4] > .__blkdev_get+0xd4/0x520 > [ 0.231138] [c000000049443690] [c000000000396cc0] .blkdev_get+0x1c0/0x4f0 > [ 0.231145] [c000000049443790] [c000000000336d64] > .do_dentry_open.isra.1+0x2a4/0x410 > [ 0.231152] [c000000049443830] [c0000000003523f4] > .path_openat+0x624/0x1580 > [ 0.231157] [c000000049443990] [c000000000354ce4] > .do_filp_open+0x84/0x120 > [ 0.231163] [c000000049443b10] [c000000000338d74] > .do_sys_open+0x214/0x300 > [ 0.231170] [c000000049443be0] [c000000000da69ac] > .md_run_setup+0xa0/0xec > [ 0.231176] [c000000049443c60] [c000000000da4fbc] > .prepare_namespace+0x60/0x240 > [ 0.231182] [c000000049443ce0] [c000000000da47a8] > .kernel_init_freeable+0x330/0x36c > [ 0.231190] [c000000049443db0] [c00000000000dc44] .kernel_init+0x24/0x160 > [ 0.231197] [c000000049443e30] [c00000000000badc] > .ret_from_kernel_thread+0x58/0x7c > [ 0.231202] Instruction dump: > [ 0.231206] 60000000 3d22ffee 89296bfb 2f890000 409effdc 3c62ffc6 39200001 > 3d42ffee > [ 0.231216] 38630928 992a6bfb 484a6e79 60000000 <0fe00000> 4bffffb8 > 60000000 60000000 > [ 0.231226] ---[ end trace 8c51f269ad91ffc2 ]--- > [ 0.231233] md: Autodetecting RAID arrays. > [ 0.231236] md: autorun ... > [ 0.231239] md: ... autorun DONE. > [ 0.234188] EXT4-fs (sda4): mounting ext3 file system using the ext4 subsystem > [ 0.250506] refcount_t: underflow; use-after-free. > [ 0.250531] ------------[ cut here ]------------ > [ 0.250537] WARNING: CPU: 0 PID: 3 at lib/refcount.c:207 > .refcount_dec_not_one+0x104/0x120 > [ 0.250542] Modules linked in: > [ 0.250546] CPU: 0 PID: 3 Comm: kworker/0:0 Tainted: G W 4.11.0-rc1- > gccN-next-20170310-g5be4921 #1 > [ 0.250553] Workqueue: events .delayed_fput > [ 0.250557] task: c000000049404900 task.stack: c000000049448000 > [ 0.250562] NIP: c0000000005ac964 LR: c0000000005ac960 CTR: > c000000000743390 > [ 0.250567] REGS: c00000004944b530 TRAP: 0700 Tainted: G W (4.11.0- > rc1-gccN-next-20170310-g5be4921) > [ 0.250572] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> > [ 0.250578] CR: 24002422 XER: 00000007 > [ 0.250584] CFAR: c000000000a5356c SOFTE: 1 > [ 0.250584] GPR00: c0000000005ac960 c00000004944b7b0 c000000001079d00 > 0000000000000026 > [ 0.250584] GPR04: 0000000000000000 0000000000000113 0000000000000000 > c0000000010418a0 > [ 0.250584] GPR08: 000000004af80000 c000000000ecc9a8 c000000000ecc9a8 > 0000000000000000 > [ 0.250584] GPR12: 0000000022002824 c000000006bb0000 c0000000001116d0 > c000000049050200 > [ 0.250584] GPR16: 0000000000000000 0000000000000000 0000000000000000 > 0000000000000000 > [ 0.250584] GPR20: 0000000000000001 0000000000000000 c000000048030a98 > 0000000000000001 > [ 0.250584] GPR24: 000000000002001d 0000000000000000 0000000000000000 > c0000000461af000 > [ 0.250584] GPR28: 0000000000000000 c000000048030bd8 c0000000461aea08 > c0000000012060b8 > [ 0.250645] NIP [c0000000005ac964] .refcount_dec_not_one+0x104/0x120 > [ 0.250650] LR [c0000000005ac960] .refcount_dec_not_one+0x100/0x120 > [ 0.250654] Call Trace: > [ 0.250658] [c00000004944b7b0] [c0000000005ac960] > .refcount_dec_not_one+0x100/0x120 (unreliable) > [ 0.250665] [c00000004944b820] [c0000000005ac9a0] > .refcount_dec_and_lock+0x20/0xc0 > [ 0.250671] [c00000004944b8a0] [c000000000870fa4] .mddev_put+0x34/0x18 0 > [ 0.250677] [c00000004944b930] [c000000000396108] > .__blkdev_put+0x288/0x350 > [ 0.250683] [c00000004944ba30] [c0000000003968f0] .blkdev_close+0x30/0x50 > [ 0.250689] [c00000004944bab0] [c00000000033e7d8] .__fput+0xc8/0x2a0 > [ 0.250695] [c00000004944bb60] [c00000000033ea08] > .delayed_fput+0x58/0x80 > [ 0.250701] [c00000004944bbe0] [c000000000107ea0] > .process_one_work+0x2a0/0x630 > [ 0.250707] [c00000004944bc80] [c0000000001082c8] > .worker_thread+0x98/0x6a0 > [ 0.250713] [c00000004944bd70] [c000000000111868] .kthread+0x198/0x1a0 > [ 0.250719] [c00000004944be30] [c00000000000badc] > .ret_from_kernel_thread+0x58/0x7c > [ 0.250724] Instruction dump: > [ 0.250728] 419e000c 38210070 4e800020 7c0802a6 3c62ffc6 39200001 > 3d42ffee 38630958 > [ 0.250738] 992a6bfe f8010080 484a6bd1 60000000 <0fe00000> e8010080 > 38600001 7c0803a6 > [ 0.250748] ---[ end trace 8c51f269ad91ffc3 ]--- > [ 0.262454] EXT4-fs (sda4): mounted filesystem with ordered data mode. Opts: > (null)
[toc] | [prev] | [next] | [standalone]
| From | James Bottomley <James.Bottomley@HansenPartnership.com> |
|---|---|
| Date | 2017-03-14 16:00 +0100 |
| Subject | Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t |
| Message-ID | <tkW5B-1Ad-53@gated-at.bofh.it> |
| In reply to | #1600289 |
On Tue, 2017-03-14 at 12:29 +0000, Reshetova, Elena wrote: > > Elena Reshetova <elena.reshetova@intel.com> writes: > > > > > refcount_t type and corresponding API should be > > > used instead of atomic_t when the variable is used as > > > a reference counter. This allows to avoid accidental > > > refcounter overflows that might lead to use-after-free > > > situations. > > > > > > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > > > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > --- > > > drivers/md/md.c | 6 +++--- > > > drivers/md/md.h | 3 ++- > > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > When booting linux-next (specifically 5be4921c9958ec) I'm seeing > > the > > backtrace below. I suspect this patch is just exposing an existing > > issue? > > Yes, we have actually been following this issue in the another > thread. > It looks like the object is re-used somehow, but I can't quite > understand how just by reading the code. > This was what I put into the previous thread: > > "The log below indicates that you are using your refcounter in a bit > weird way in mddev_find(). > However, I can't find the place (just by reading the code) where you > would increment refcounter from zero (vs. setting it to one). > It looks like you either iterate over existing nodes (and increment > their counters, which should be >= 1 at the time of increment) or > create a new node, but then mddev_init() sets the counter to 1. " > > If you can help to understand what is going on with the object > creation/destruction, would be appreciated! > > Also Shaohua Li stopped this patch coming from his tree since the > issue was caught at that time, so we are not going to merge this > until we figure it out. Asking on the correct list (dm-devel) would have got you the easy answer: The refcount behind mddev->active is a genuine atomic. It has refcount properties but only if the array fails to initialise (in that case, final put kills it). Once it's added to the system as a gendisk, it cannot be freed until md_free(). Thus its ->active count can go to zero (when it becomes inactive; usually because of an unmount). On a simple allocation regardless of outcome, the last executed statement in md_alloc is mddev_put(): that destroys the device if we didn't manage to create it or returns 0 and adds an inactive device to the system which the user can get with mddev_find(). James
[toc] | [prev] | [next] | [standalone]
| From | "Reshetova, Elena" <elena.reshetova@intel.com> |
|---|---|
| Date | 2017-03-16 19:10 +0100 |
| Subject | RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t |
| Message-ID | <tlI0y-25v-13@gated-at.bofh.it> |
| In reply to | #1600539 |
> On Tue, 2017-03-14 at 12:29 +0000, Reshetova, Elena wrote: > > > Elena Reshetova <elena.reshetova@intel.com> writes: > > > > > > > refcount_t type and corresponding API should be > > > > used instead of atomic_t when the variable is used as > > > > a reference counter. This allows to avoid accidental > > > > refcounter overflows that might lead to use-after-free > > > > situations. > > > > > > > > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > > > > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > > --- > > > > drivers/md/md.c | 6 +++--- > > > > drivers/md/md.h | 3 ++- > > > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > > > When booting linux-next (specifically 5be4921c9958ec) I'm seeing > > > the > > > backtrace below. I suspect this patch is just exposing an existing > > > issue? > > > > Yes, we have actually been following this issue in the another > > thread. > > It looks like the object is re-used somehow, but I can't quite > > understand how just by reading the code. > > This was what I put into the previous thread: > > > > "The log below indicates that you are using your refcounter in a bit > > weird way in mddev_find(). > > However, I can't find the place (just by reading the code) where you > > would increment refcounter from zero (vs. setting it to one). > > It looks like you either iterate over existing nodes (and increment > > their counters, which should be >= 1 at the time of increment) or > > create a new node, but then mddev_init() sets the counter to 1. " > > > > If you can help to understand what is going on with the object > > creation/destruction, would be appreciated! > > > > Also Shaohua Li stopped this patch coming from his tree since the > > issue was caught at that time, so we are not going to merge this > > until we figure it out. > > Asking on the correct list (dm-devel) would have got you the easy > answer: The refcount behind mddev->active is a genuine atomic. It has > refcount properties but only if the array fails to initialise (in that > case, final put kills it). Once it's added to the system as a gendisk, > it cannot be freed until md_free(). Thus its ->active count can go to > zero (when it becomes inactive; usually because of an unmount). On a > simple allocation regardless of outcome, the last executed statement in > md_alloc is mddev_put(): that destroys the device if we didn't manage > to create it or returns 0 and adds an inactive device to the system > which the user can get with mddev_find(). Thank you James for explaining this! I guess in this case, the conversion doesn't make sense. And sorry about not asking in a correct place: we are handling many similar patches now and while I try to reach the right audience using get_maintainer script, it doesn't always succeeds. Best Regards, Elena. > > James >
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web