[PATCH] target/tcm_fc: Fix use-after-free of ft_tpg

[PATCH] target/tcm_fc: Fix use-after-free of ft_tpg

[Andy Grover]

ft_del_tpg checks tpg->tport is set before unlinking the tpg from the
tport when the tpg is being removed. Set this pointer in ft_tport_create,
or the unlinking won't happen in ft_del_tpg and tport->tpg will reference
a deleted object.

see https://bugzilla.redhat.com/show_bug.cgi?id=1071340

Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Andy Grover <agrover-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/target/tcm_fc/tfc_sess.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c
index 0475142..1329726 100644
--- a/drivers/target/tcm_fc/tfc_sess.c
+++ b/drivers/target/tcm_fc/tfc_sess.c
@@ -68,6 +68,7 @@ static struct ft_tport *ft_tport_create(struct fc_lport *lport)
 
 	if (tport) {
 		tport->tpg = tpg;
+		tpg->tport = tport;
 		return tport;
 	}
 
-- 
1.9.0

Re: [PATCH] target/tcm_fc: Fix use-after-free of ft_tpg

[Nicholas A. Bellinger]

Hi Andy,

On Fri, 2014-04-04 at 16:44 -0700, Andy Grover wrote:
> ft_del_tpg checks tpg->tport is set before unlinking the tpg from the
> tport when the tpg is being removed. Set this pointer in ft_tport_create,
> or the unlinking won't happen in ft_del_tpg and tport->tpg will reference
> a deleted object.
> 
> see https://bugzilla.redhat.com/show_bug.cgi?id=1071340
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Grover <agrover@redhat.com>
> ---
>  drivers/target/tcm_fc/tfc_sess.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c
> index 0475142..1329726 100644
> --- a/drivers/target/tcm_fc/tfc_sess.c
> +++ b/drivers/target/tcm_fc/tfc_sess.c
> @@ -68,6 +68,7 @@ static struct ft_tport *ft_tport_create(struct fc_lport *lport)
>  
>  	if (tport) {
>  		tport->tpg = tpg;
> +		tpg->tport = tport;
>  		return tport;
>  	}
>  

I'm confused how this addresses the bug above..?

Does this mean the bug is only triggered when a previously
configured /sys/kernel/config/target/fc/$FCOE_WWPN/$TPGT has been
removed, and then a new PLOGI picks up a left-over pointer to an
endpoint that previously had been released..?

Care to add a more detailed commit explaining the exact details..?

--nab

[PATCH] target/tcm_fc: Fix use-after-free of ft_tpg

[Andy Grover]

This can happen:

- lport created, tport (our per-lport, per-provider context) is allocated.
  tport->tpg = NULL
- tpg created
- a PRLI is received. ft_tport_create is called, tpg is found and
  tport->tpg is set
- tpg removed. ft_tpg is freed in ft_del_tpg. Since tpg->tport was not
  set, tport->tpg is not cleared and points at freed memory
- Future calls to ft_tport_create return tport via first conditional,
  instead of searching for new tpg by calling ft_lport_find_tpg.
  tport->tpg is still invalid, and will access freed memory.

This patch sets tpg->tport in ft_tport_create, because that's what
ft_del_tpg checks, and is the only way to get back to the tport to
clear tport->tpg.

see https://bugzilla.redhat.com/show_bug.cgi?id=1071340

Cc: stable@vger.kernel.org
Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/tcm_fc/tfc_sess.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c
index 0475142..1329726 100644
--- a/drivers/target/tcm_fc/tfc_sess.c
+++ b/drivers/target/tcm_fc/tfc_sess.c
@@ -68,6 +68,7 @@ static struct ft_tport *ft_tport_create(struct fc_lport *lport)
 
 	if (tport) {
 		tport->tpg = tpg;
+		tpg->tport = tport;
 		return tport;
 	}
 
-- 
1.9.0