We need to move the kfree() down a line so we don't dereference a freed
variable.
Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to target_alloc_session')
Signed-off-by: Dan Carpenter <[email protected]>
diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
index 7276a73..e352a31 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1596,8 +1596,8 @@ static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
#define MAKE_NEXUS_MSG "core_tpg_check_initiator_node_acl() failed for %s\n"
pr_debug(MAKE_NEXUS_MSG, name);
#undef MAKE_NEXUS_MSG
- kfree(tv_nexus);
ret = PTR_ERR(tv_nexus->tvn_se_sess);
+ kfree(tv_nexus);
}
out_unlock:
Dan Carpenter <[email protected]> writes:
> We need to move the kfree() down a line so we don't dereference a freed
> variable.
>
> Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to target_alloc_session')
> Signed-off-by: Dan Carpenter <[email protected]>
It's okay to take this via target:
Signed-off-by: Felipe Balbi <[email protected]>
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 7276a73..e352a31 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -1596,8 +1596,8 @@ static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
> #define MAKE_NEXUS_MSG "core_tpg_check_initiator_node_acl() failed for %s\n"
> pr_debug(MAKE_NEXUS_MSG, name);
> #undef MAKE_NEXUS_MSG
> - kfree(tv_nexus);
> ret = PTR_ERR(tv_nexus->tvn_se_sess);
> + kfree(tv_nexus);
> }
>
> out_unlock:
--
balbi
On Wed, 2016-03-02 at 13:08 +0300, Dan Carpenter wrote:
> We need to move the kfree() down a line so we don't dereference a freed
> variable.
>
> Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to target_alloc_session')
> Signed-off-by: Dan Carpenter <[email protected]>
>
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 7276a73..e352a31 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -1596,8 +1596,8 @@ static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
> #define MAKE_NEXUS_MSG "core_tpg_check_initiator_node_acl() failed for %s\n"
> pr_debug(MAKE_NEXUS_MSG, name);
> #undef MAKE_NEXUS_MSG
> - kfree(tv_nexus);
> ret = PTR_ERR(tv_nexus->tvn_se_sess);
> + kfree(tv_nexus);
> }
>
> out_unlock:
Fixed + squashed into the original patch.
Thanks Dan.
Hi Felipe + usb-gadget folks,
On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:
> Dan Carpenter <[email protected]> writes:
> > We need to move the kfree() down a line so we don't dereference a freed
> > variable.
> >
> > Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to target_alloc_session')
> > Signed-off-by: Dan Carpenter <[email protected]>
>
> It's okay to take this via target:
>
> Signed-off-by: Felipe Balbi <[email protected]>
>
Note this specific patch is only a mechanical change, and we still need
reviews for the more interesting conversions:
usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
http://www.spinics.net/lists/target-devel/msg11777.html
usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
http://www.spinics.net/lists/target-devel/msg11782.html
Felipe, Sebastian, & Andrezj, would you be so kind to review and test
usb-gadget using target-pending/for-next code..?
Hi,
"Nicholas A. Bellinger" <[email protected]> writes:
> [ text/plain ]
> Hi Felipe + usb-gadget folks,
>
> On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:
>> Dan Carpenter <[email protected]> writes:
>> > We need to move the kfree() down a line so we don't dereference a freed
>> > variable.
>> >
>> > Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to target_alloc_session')
>> > Signed-off-by: Dan Carpenter <[email protected]>
>>
>> It's okay to take this via target:
>>
>> Signed-off-by: Felipe Balbi <[email protected]>
>>
>
> Note this specific patch is only a mechanical change, and we still need
> reviews for the more interesting conversions:
>
> usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
> http://www.spinics.net/lists/target-devel/msg11777.html
>
> usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
> http://www.spinics.net/lists/target-devel/msg11782.html
>
I don't have either patch in my inbox apparently. Care to resend ?
sorry
--
balbi
Hi Nicholas,
W dniu 05.03.2016 o 08:26, Nicholas A. Bellinger pisze:
> Hi Felipe + usb-gadget folks,
>
> On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:
<snip>
>
> usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
> http://www.spinics.net/lists/target-devel/msg11777.html
>
> usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
> http://www.spinics.net/lists/target-devel/msg11782.html
>
> Felipe, Sebastian, & Andrezj, would you be so kind to review and test
> usb-gadget using target-pending/for-next code..?
>
>
Actually I have noticed a problem at
8b00965 "target: Convert demo-mode only drivers to target_alloc_session"
I get:
[ 1698.406304] configfs-gadget gadget: high-speed config #1: c
[ 1698.410547] Using the BOT protocol
[ 1698.414163] Missing nexus, ignoring command
[ 1698.417944] bot_cmd_complete(309): -22
I think the third message is from f_tcm. It is because
tpg->tpg_nexus is not set anymore, because the line
tpg->tpg_nexus = tv_nexus;
is removed by the commit mentioned above.
Restoring this line fixes the problem.
Then percpu_ida commit produces the below result
(it does not happen immediately, but a while after
running the script).
I did not bisect, though; I only checked the commits
which alter drivers/usb/gadget/function/f_tcm.c.
The last one which (almost) works is:
8b00965 "target: Convert demo-mode only drivers to target_alloc_session"
AP
I used the following script:
8<----------------------------
#!/bin/bash
mount -t configfs none /sys/kernel/config
modprobe libcomposite
cd /sys/kernel/config/usb_gadget
mkdir tcm
cd tcm
mkdir functions/tcm.0
cd /sys/kernel/config/target/
mkdir usb_gadget
cd usb_gadget
mkdir naa.0123456789abcdef
cd naa.0123456789abcdef
mkdir tpgt_1
cd tpgt_1
echo naa.01234567890abcdef > nexus
echo 1 > enable
cd /sys/kernel/config/usb_gadget/tcm
mkdir configs/c.1
ln -s functions/tcm.0 configs/c.1
echo 0x0525 > idVendor
echo 0x1234 > idProduct
echo 12400000.dwc3 > UDC
8<----------------------------
# [ 45.510513] ERR bot_status_complete(73)
[ 45.671921] configfs-gadget gadget: high-speed config #1: c
[ 45.676158] Using the BOT protocol
[ 45.679860] ------------[ cut here ]------------
[ 45.683981] kernel BUG at drivers/target/target_core_transport.c:1476!
[ 45.690480] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
[ 45.696285] Modules linked in: usb_f_tcm libcomposite
[ 45.701316] CPU: 2 PID: 1221 Comm: kworker/2:1 Not tainted 4.5.0-rc5+ #171
[ 45.708156] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 45.714258] Workqueue: tcm_usb_gadget bot_cmd_work [usb_f_tcm]
[ 45.720030] task: edc1f6c0 ti: ee2d6000 task.ti: ee2d6000
[ 45.725410] PC is at target_submit_cmd_map_sgls+0x1fc/0x20c
[ 45.730947] LR is at 0xeea71400
[ 45.734070] pc : [<c0326a00>] lr : [<eea71400>] psr: a0000013
[ 45.734070] sp : ee2d7e50 ip : 00000000 fp : c07f8100
[ 45.745505] r10: 00000000 r9 : eefa25c0 r8 : ed593988
[ 45.750705] r7 : ed593b8c r6 : ee00d830 r5 : 00000000 r4 : ed5939e0
[ 45.757204] r3 : bf0144d8 r2 : ed593988 r1 : eea71400 r0 : ed5939e0
[ 45.763705] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 45.770809] Control: 10c5387d Table: 6d44806a DAC: 00000051
[ 45.776528] Process kworker/2:1 (pid: 1221, stack limit = 0xee2d6210)
[ 45.782940] Stack: (0xee2d7e50 to 0xee2d8000)
[ 45.787276] 7e40: 002293a6 00000000 00000000 00000000
[ 45.795423] 7e60: 00000000 00000000 00000003 00000020 00000000 c0326a60 00000000 00000000
[ 45.803568] 7e80: 00000000 00000020 00000003 00000000 00000000 00000000 00000000 00000000
[ 45.811714] 7ea0: 00000000 00000000 ed5939cc 00000020 ed5939e0 edf6b580 00000000 bf012358
[ 45.819860] 7ec0: 00000000 00000000 00000000 00000020 00000003 00000000 eefa2ac0 ee30da00
[ 45.828006] 7ee0: ed5939cc eefa25c0 eefaaa00 c00369bc edc1f6c0 ee30da00 00000001 ee30da00
[ 45.836151] 7f00: eefa25e4 00000001 eefa25c0 ee30da18 eefa25c0 00000008 c07f8100 c0036c24
[ 45.844297] 7f20: edc1f6c0 eeafefc0 00000000 eeafefc0 00000000 ee30da00 c0036bf8 00000000
[ 45.852442] 7f40: 00000000 00000000 00000000 c003bdf8 fffffffe 00000000 ee2d7f70 ee30da00
[ 45.860587] 7f60: 00000000 00000000 dead4ead ffffffff ffffffff ee2d7f74 ee2d7f74 00000000
[ 45.868733] 7f80: 00000000 dead4ead ffffffff ffffffff ee2d7f90 ee2d7f90 ee2d7fac eeafefc0
[ 45.876878] 7fa0: c003bd20 00000000 00000000 c000f8b8 00000000 00000000 00000000 00000000
[ 45.885023] 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 45.893169] 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
[ 45.901324] [<c0326a00>] (target_submit_cmd_map_sgls) from [<c0326a60>] (target_submit_cmd+0x50/0x58)
[ 45.910514] [<c0326a60>] (target_submit_cmd) from [<bf012358>] (bot_cmd_work+0x74/0x100 [usb_f_tcm])
[ 45.919623] [<bf012358>] (bot_cmd_work [usb_f_tcm]) from [<c00369bc>] (process_one_work+0x120/0x324)
[ 45.928703] [<c00369bc>] (process_one_work) from [<c0036c24>] (worker_thread+0x2c/0x4b4)
[ 45.936760] [<c0036c24>] (worker_thread) from [<c003bdf8>] (kthread+0xd8/0xf4)
[ 45.943955] [<c003bdf8>] (kthread) from [<c000f8b8>] (ret_from_fork+0x14/0x3c)
[ 45.951142] Code: ebf3d01a eaffffd0 ebfbc490 eafffff8 (e7f001f2)
[ 45.957206] ---[ end trace 21ce9edfaf719456 ]---
[ 45.962132] Unable to handle kernel paging request at virtual address ffffffe0
[ 45.968986] pgd = c0004000
[ 45.971672] [ffffffe0] *pgd=6fffd861, *pte=00000000, *ppte=00000000
[ 45.977913] Internal error: Oops: 37 [#2] PREEMPT SMP ARM
[ 45.983282] Modules linked in: usb_f_tcm libcomposite
[ 45.988313] CPU: 2 PID: 1221 Comm: kworker/2:1 Tainted: G D 4.5.0-rc5+ #171
[ 45.996367] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 46.002441] task: edc1f6c0 ti: ee2d6000 task.ti: ee2d6000
[ 46.007810] PC is at kthread_data+0x4/0xc
[ 46.011797] LR is at wq_worker_sleeping+0xc/0xd8
[ 46.016389] pc : [<c003c42c>] lr : [<c0037ac8>] psr: 00000093
[ 46.016389] sp : ee2d7cc8 ip : 00000000 fp : ee2d7cf4
[ 46.027824] r10: c07f4ac0 r9 : 00000000 r8 : 00000002
[ 46.033024] r7 : c07f85b8 r6 : edc1fa0c r5 : edc1f6c0 r4 : 00000002
[ 46.039523] r3 : 00000000 r2 : 00000000 r1 : 00000002 r0 : edc1f6c0
[ 46.046023] Flags: nzcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
[ 46.053214] Control: 10c5387d Table: 6d44806a DAC: 00000051
[ 46.058933] Process kworker/2:1 (pid: 1221, stack limit = 0xee2d6210)
[ 46.065345] Stack: (0xee2d7cc8 to 0xee2d8000)
[ 46.069683] 7cc0: eefa2ac0 c054f544 00000002 ee2d7a2c ee888000 edc1f978
[ 46.077829] 7ce0: ee2d7d00 00000000 edc1f6c0 00000001 ee2d7cfc c054f92c edc1f6c0 c0025404
[ 46.085975] 7d00: ee2d7d00 ee2d7d00 00000001 c07fc4b4 c0834404 0000000b c06a1620 00000000
[ 46.094120] 7d20: edc1f6c0 60000093 c07f8100 c00125d8 ee2d6210 0000000b 07f001f2 c07fc4c8
[ 46.102266] 7d40: e7f001f2 ee2d7e00 c0326a00 c0012f20 00000000 c00090bc 00000006 ffff0a10
[ 46.110411] 7d60: 00000004 00000000 00030001 c0326a00 00000600 ffff0a10 00000000 c083db90
[ 46.118557] 7d80: c08366ac 00000000 c08366ac 00000001 c083dba0 c083db90 00000016 c0222ae8
[ 46.126702] 7da0: ffff3a11 ee2d7db0 c00600d0 c06a862c 0000002d 000a513e 00000002 20000093
[ 46.134848] 7dc0: c07ffe1c 0000015e 00000000 c08361a0 00000006 c0553e58 00000002 0000015e
[ 46.142993] 7de0: 00000000 c0326a04 00000000 c0013358 00000000 e7100000 ee2d6000 c0012f20
[ 46.151139] 7e00: ed5939e0 eea71400 ed593988 bf0144d8 ed5939e0 00000000 ee00d830 ed593b8c
[ 46.159284] 7e20: ed593988 eefa25c0 00000000 c07f8100 00000000 ee2d7e50 eea71400 c0326a00
[ 46.167430] 7e40: a0000013 ffffffff 00000051 c00504a0 002293a6 00000000 00000000 00000000
[ 46.175575] 7e60: 00000000 00000000 00000003 00000020 00000000 c0326a60 00000000 00000000
[ 46.183720] 7e80: 00000000 00000020 00000003 00000000 00000000 00000000 00000000 00000000
[ 46.191866] 7ea0: 00000000 00000000 ed5939cc 00000020 ed5939e0 edf6b580 00000000 bf012358
[ 46.200011] 7ec0: 00000000 00000000 00000000 00000020 00000003 00000000 eefa2ac0 ee30da00
[ 46.208157] 7ee0: ed5939cc eefa25c0 eefaaa00 c00369bc edc1f6c0 ee30da00 00000001 ee30da00
[ 46.216302] 7f00: eefa25e4 00000001 eefa25c0 ee30da18 eefa25c0 00000008 c07f8100 c0036c24
[ 46.224448] 7f20: edc1f6c0 eeafefc0 00000000 eeafefc0 00000000 ee30da00 c0036bf8 00000000
[ 46.232593] 7f40: 00000000 00000000 00000000 c003bdf8 fffffffe 00000000 ee2d7f70 ee30da00
[ 46.240739] 7f60: 00000000 00000000 dead4ead ffffffff ffffffff ee2d7f74 ee2d7f74 00000001
[ 46.248885] 7f80: 00010001 dead4ead ffffffff ffffffff ee2d7f90 ee2d7f90 ee2d7fac eeafefc0
[ 46.257029] 7fa0: c003bd20 00000000 00000000 c000f8b8 00000000 00000000 00000000 00000000
[ 46.265175] 7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 46.273321] 7fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
[ 46.281472] [<c003c42c>] (kthread_data) from [<c0037ac8>] (wq_worker_sleeping+0xc/0xd8)
[ 46.289450] [<c0037ac8>] (wq_worker_sleeping) from [<c054f544>] (__schedule+0x260/0x450)
[ 46.297503] [<c054f544>] (__schedule) from [<c054f92c>] (schedule+0x40/0xac)
[ 46.304521] [<c054f92c>] (schedule) from [<c0025404>] (do_exit+0x620/0x9c4)
[ 46.311454] [<c0025404>] (do_exit) from [<c00125d8>] (die+0x20c/0x2e0)
[ 46.317950] [<c00125d8>] (die) from [<c00090bc>] (do_undefinstr+0xb8/0x220)
[ 46.324881] [<c00090bc>] (do_undefinstr) from [<c0012f20>] (__und_svc_finish+0x0/0x20)
[ 46.332760] Exception stack(0xee2d7e00 to 0xee2d7e48)
[ 46.337792] 7e00: ed5939e0 eea71400 ed593988 bf0144d8 ed5939e0 00000000 ee00d830 ed593b8c
[ 46.345937] 7e20: ed593988 eefa25c0 00000000 c07f8100 00000000 ee2d7e50 eea71400 c0326a00
[ 46.354077] 7e40: a0000013 ffffffff
[ 46.357552] [<c0012f20>] (__und_svc_finish) from [<c0326a00>] (target_submit_cmd_map_sgls+0x1fc/0x20c)
[ 46.366822] [<c0326a00>] (target_submit_cmd_map_sgls) from [<c0326a60>] (target_submit_cmd+0x50/0x58)
[ 46.376023] [<c0326a60>] (target_submit_cmd) from [<bf012358>] (bot_cmd_work+0x74/0x100 [usb_f_tcm])
[ 46.385121] [<bf012358>] (bot_cmd_work [usb_f_tcm]) from [<c00369bc>] (process_one_work+0x120/0x324)
[ 46.394206] [<c00369bc>] (process_one_work) from [<c0036c24>] (worker_thread+0x2c/0x4b4)
[ 46.402264] [<c0036c24>] (worker_thread) from [<c003bdf8>] (kthread+0xd8/0xf4)
[ 46.409456] [<c003bdf8>] (kthread) from [<c000f8b8>] (ret_from_fork+0x14/0x3c)
[ 46.416647] Code: c003ba54 c06a4010 c0835c50 e5903310 (e5130020)
[ 46.422710] ---[ end trace 21ce9edfaf719457 ]---
[ 46.427299] Fixing recursive fault but reboot is needed!
Hi Andrzej,
On Wed, 2016-03-09 at 13:53 +0100, Andrzej Pietrasiewicz wrote:
> Hi Nicholas,
>
> W dniu 05.03.2016 o 08:26, Nicholas A. Bellinger pisze:
> > Hi Felipe + usb-gadget folks,
> >
> > On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:
>
> <snip>
>
> >
> > usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
> > http://www.spinics.net/lists/target-devel/msg11777.html
> >
> > usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
> > http://www.spinics.net/lists/target-devel/msg11782.html
> >
> > Felipe, Sebastian, & Andrezj, would you be so kind to review and test
> > usb-gadget using target-pending/for-next code..?
> >
> >
>
> Actually I have noticed a problem at
> 8b00965 "target: Convert demo-mode only drivers to target_alloc_session"
>
> I get:
>
> [ 1698.406304] configfs-gadget gadget: high-speed config #1: c
> [ 1698.410547] Using the BOT protocol
> [ 1698.414163] Missing nexus, ignoring command
> [ 1698.417944] bot_cmd_complete(309): -22
>
> I think the third message is from f_tcm. It is because
> tpg->tpg_nexus is not set anymore, because the line
>
> tpg->tpg_nexus = tv_nexus;
>
> is removed by the commit mentioned above.
>
> Restoring this line fixes the problem.
Thanks for testing!
Applying the following patch to re-add the missing assingment
as a proper alloc_session callback.
diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
index e352a31..348140c 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1570,6 +1570,16 @@ out:
return ret;
}
+static int usbg_alloc_sess_cb(struct se_portal_group *se_tpg,
+ struct se_session *se_sess, void *p)
+{
+ struct usbg_tpg *tpg = container_of(se_tpg,
+ struct usbg_tpg, se_tpg);
+
+ tpg->tpg_nexus = p;
+ return 0;
+}
+
static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
{
struct tcm_usbg_nexus *tv_nexus;
@@ -1591,7 +1601,7 @@ static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
tv_nexus->tvn_se_sess = target_alloc_session(&tpg->se_tpg, 128,
sizeof(struct usbg_cmd),
TARGET_PROT_NORMAL, name,
- tv_nexus, NULL);
+ tv_nexus, usbg_alloc_sess_cb);
if (IS_ERR(tv_nexus->tvn_se_sess)) {
#define MAKE_NEXUS_MSG "core_tpg_check_initiator_node_acl() failed for %s\n"
pr_debug(MAKE_NEXUS_MSG, name);
>
> Then percpu_ida commit produces the below result
> (it does not happen immediately, but a while after
> running the script).
> I did not bisect, though; I only checked the commits
> which alter drivers/usb/gadget/function/f_tcm.c.
> The last one which (almost) works is:
>
> 8b00965 "target: Convert demo-mode only drivers to target_alloc_session"
>
> AP
>
> I used the following script:
>
> 8<----------------------------
>
> #!/bin/bash
>
> mount -t configfs none /sys/kernel/config
> modprobe libcomposite
> cd /sys/kernel/config/usb_gadget
> mkdir tcm
> cd tcm
> mkdir functions/tcm.0
> cd /sys/kernel/config/target/
> mkdir usb_gadget
> cd usb_gadget
> mkdir naa.0123456789abcdef
> cd naa.0123456789abcdef
> mkdir tpgt_1
> cd tpgt_1
> echo naa.01234567890abcdef > nexus
> echo 1 > enable
> cd /sys/kernel/config/usb_gadget/tcm
> mkdir configs/c.1
> ln -s functions/tcm.0 configs/c.1
> echo 0x0525 > idVendor
> echo 0x1234 > idProduct
> echo 12400000.dwc3 > UDC
>
> 8<----------------------------
>
> # [ 45.510513] ERR bot_status_complete(73)
> [ 45.671921] configfs-gadget gadget: high-speed config #1: c
> [ 45.676158] Using the BOT protocol
> [ 45.679860] ------------[ cut here ]------------
> [ 45.683981] kernel BUG at drivers/target/target_core_transport.c:1476!
Mmmm, usbg_get_cmd() was missing an explicit memset() after tag lookup.
How about the following..?
diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
index e352a31..d4e8a91 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1078,6 +1078,7 @@ static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,
return ERR_PTR(-ENOMEM);
cmd = &((struct usbg_cmd *)se_sess->sess_cmd_map)[tag];
+ memset(cmd, 0, sizeof(*cmd));
cmd->se_cmd.map_tag = tag;
cmd->se_cmd.tag = cmd->tag = scsi_tag;
cmd->fu = fu;
On Wed, 2016-03-09 at 13:38 +0200, Felipe Balbi wrote:
> Hi,
>
> "Nicholas A. Bellinger" <[email protected]> writes:
> > [ text/plain ]
> > Hi Felipe + usb-gadget folks,
> >
> > On Wed, 2016-03-02 at 13:55 +0200, Felipe Balbi wrote:
> >> Dan Carpenter <[email protected]> writes:
> >> > We need to move the kfree() down a line so we don't dereference a freed
> >> > variable.
> >> >
> >> > Fixes: 1b418a8fcbc0 ('target: Convert demo-mode only drivers to target_alloc_session')
> >> > Signed-off-by: Dan Carpenter <[email protected]>
> >>
> >> It's okay to take this via target:
> >>
> >> Signed-off-by: Felipe Balbi <[email protected]>
> >>
> >
> > Note this specific patch is only a mechanical change, and we still need
> > reviews for the more interesting conversions:
> >
> > usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
> > http://www.spinics.net/lists/target-devel/msg11777.html
> >
> > usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
> > http://www.spinics.net/lists/target-devel/msg11782.html
> >
> I don't have either patch in my inbox apparently. Care to resend ?
>
> sorry
>
A -v4 is going on this week, and will make sure they hit your inbox.
Hi Nicholas,
W dniu 10.03.2016 o 06:19, Nicholas A. Bellinger pisze:
> Hi Andrzej,
>
> On Wed, 2016-03-09 at 13:53 +0100, Andrzej Pietrasiewicz wrote:
>> Hi Nicholas,
>>
<snip>
>
> Applying the following patch to re-add the missing assingment
> as a proper alloc_session callback.
>
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index e352a31..348140c 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -1570,6 +1570,16 @@ out:
> return ret;
> }
>
> +static int usbg_alloc_sess_cb(struct se_portal_group *se_tpg,
> + struct se_session *se_sess, void *p)
> +{
> + struct usbg_tpg *tpg = container_of(se_tpg,
> + struct usbg_tpg, se_tpg);
> +
> + tpg->tpg_nexus = p;
> + return 0;
> +}
> +
> static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
> {
> struct tcm_usbg_nexus *tv_nexus;
> @@ -1591,7 +1601,7 @@ static int tcm_usbg_make_nexus(struct usbg_tpg *tpg, char *name)
> tv_nexus->tvn_se_sess = target_alloc_session(&tpg->se_tpg, 128,
> sizeof(struct usbg_cmd),
> TARGET_PROT_NORMAL, name,
> - tv_nexus, NULL);
> + tv_nexus, usbg_alloc_sess_cb);
> if (IS_ERR(tv_nexus->tvn_se_sess)) {
> #define MAKE_NEXUS_MSG "core_tpg_check_initiator_node_acl() failed for %s\n"
> pr_debug(MAKE_NEXUS_MSG, name);
>
<snip>
>
> Mmmm, usbg_get_cmd() was missing an explicit memset() after tag lookup.
>
> How about the following..?
>
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index e352a31..d4e8a91 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -1078,6 +1078,7 @@ static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,
> return ERR_PTR(-ENOMEM);
>
> cmd = &((struct usbg_cmd *)se_sess->sess_cmd_map)[tag];
> + memset(cmd, 0, sizeof(*cmd));
> cmd->se_cmd.map_tag = tag;
> cmd->se_cmd.tag = cmd->tag = scsi_tag;
> cmd->fu = fu;
>
>
>
I tested it. Works for me.
AP
On Thu, 2016-03-10 at 09:34 +0100, Andrzej Pietrasiewicz wrote:
> Hi Nicholas,
>
> W dniu 10.03.2016 o 06:19, Nicholas A. Bellinger pisze:
> > Hi Andrzej,
> >
> > On Wed, 2016-03-09 at 13:53 +0100, Andrzej Pietrasiewicz wrote:
> >> Hi Nicholas,
> >>
<SNIP>
> >
> > Mmmm, usbg_get_cmd() was missing an explicit memset() after tag lookup.
> >
> > How about the following..?
> >
> > diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> > index e352a31..d4e8a91 100644
> > --- a/drivers/usb/gadget/function/f_tcm.c
> > +++ b/drivers/usb/gadget/function/f_tcm.c
> > @@ -1078,6 +1078,7 @@ static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu,
> > return ERR_PTR(-ENOMEM);
> >
> > cmd = &((struct usbg_cmd *)se_sess->sess_cmd_map)[tag];
> > + memset(cmd, 0, sizeof(*cmd));
> > cmd->se_cmd.map_tag = tag;
> > cmd->se_cmd.tag = cmd->tag = scsi_tag;
> > cmd->fu = fu;
> >
> >
> >
>
> I tested it. Works for me.
Folding this missing memset() into usb-gadget's percpu_ida conversion
for -v4.
Thanks Andrzej!