linux-kernel - Re: [patch] sbp-target: checking for NULL instead of IS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 10 Mar 2016 20:33:50 -0800
From:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
To:	Chris Boot <bootc@...tc.net>
Cc:	Dan Carpenter <dan.carpenter@...cle.com>,
	target-devel@...r.kernel.org,
	linux1394-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [patch] sbp-target: checking for NULL instead of IS_ERR

On Thu, 2016-03-10 at 22:58 +0000, Chris Boot wrote:
> On 10/03/16 21:52, Chris Boot wrote:
> > On 10/03/16 20:56, Chris Boot wrote:
> >> On 05/03/16 09:33, Nicholas A. Bellinger wrote:
> >>> On Sat, 2016-03-05 at 08:45 +0000, Chris Boot wrote:
> >>>> Are these in linux-next or another branch somewhere I can easily clone
> >>>> them from?
> >>>
> >>> The patch series is in target-pending/for-next.
> >>
> >> Hi Nic,
> >>
> >> I've just managed to resurrect a test rig for this (the hardware I had
> >> for it has stopped being usable, yay!), and my initial testing shows the
> >> updated code panics on the first submitted IO.
> > 
> > So this isn't the first IO, it's exactly the 2nd IO. I'm hitting
> > BUG_ON(se_cmd->se_tfo || se_cmd->se_sess) in target_submit_cmd_map_sgls().
> > 
> > I'm assuming the se_cmd is being reused due to percpu ida allocator, and
> > the code must be missing something to clean up the se_cmd sufficiently
> > once we're done with it.
> > 
> > At this point I'm out of my depth going through the target core, so I'd
> > appreciate some pointers to get any further!
> 
> Replying to myself again... Worked it out after reading the thread
> about the usb gadget target. Here's the patch you want to squash into
> your existing series:
> 
> diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
> index a04b0605f8d0..d021997cc837 100644
> --- a/drivers/target/sbp/sbp_target.c
> +++ b/drivers/target/sbp/sbp_target.c
> @@ -933,6 +933,7 @@ static struct sbp_target_request *sbp_mgt_get_req(struct sbp_session *sess,
>                 return ERR_PTR(-ENOMEM);
>  
>         req = &((struct sbp_target_request *)se_sess->sess_cmd_map)[tag];
> +       memset(req, 0, sizeof(*req));
>         req->se_cmd.map_tag = tag;
>         req->se_cmd.tag = next_orb;
>  
> @@ -1619,12 +1620,8 @@ static void sbp_mgt_agent_rw(struct fw_card *card,
>                         rcode = RCODE_CONFLICT_ERROR;
>                         goto out;
>                 }
> -               // XXX:
> -#if 0
> -               req = sbp_mgt_get_req(agent->login->sess, card);
> -#else
> +
>                 req = kzalloc(sizeof(*req), GFP_ATOMIC);
> -#endif
>                 if (!req) {
>                         rcode = RCODE_CONFLICT_ERROR;
>                         goto out;
> 
> I hope Thunderbird hasn't mangled this too badly.
> 
> With this applied, please add this to the patch for sbp_target:
> 
> Acked-by: Chris Boot <bootc@...tc.net>
> 

Applied to target-pending/for-next, and squashing into original patches
for -v4.

Thanks BootC!