[bug report] scsi: libfc: Free skb in fc_disc_gpn_id_resp() for valid cases

[Dan Carpenter <dan.carpenter@oracle.com>]

Hello Javed Hasan,

The patch ec007ef40abb: "scsi: libfc: Free skb in
fc_disc_gpn_id_resp() for valid cases" from Jul 29, 2020, leads to
the following static checker warning:

	drivers/scsi/libfc/fc_disc.c:638 fc_disc_gpn_id_resp()
	warn: '&fp->skb' double freed

drivers/scsi/libfc/fc_disc.c
   568  static void fc_disc_gpn_id_resp(struct fc_seq *sp, struct fc_frame *fp,
   569                                  void *rdata_arg)
   570  {
   571          struct fc_rport_priv *rdata = rdata_arg;
   572          struct fc_rport_priv *new_rdata;
   573          struct fc_lport *lport;
   574          struct fc_disc *disc;
   575          struct fc_ct_hdr *cp;
   576          struct fc_ns_gid_pn *pn;
   577          u64 port_name;
   578  
   579          lport = rdata->local_port;
   580          disc = &lport->disc;
   581  
   582          if (PTR_ERR(fp) == -FC_EX_CLOSED)
   583                  goto out;
   584          if (IS_ERR(fp)) {
   585                  mutex_lock(&disc->disc_mutex);
   586                  fc_disc_restart(disc);
   587                  mutex_unlock(&disc->disc_mutex);

This call to fc_disc_restart(disc); was added in the commit, but it
wasn't mentioned in the commit message so I suspect it was committed by
mistake.

   588                  goto out;
   589          }
   590  
   591          cp = fc_frame_payload_get(fp, sizeof(*cp));
   592          if (!cp)
   593                  goto redisc;
   594          if (ntohs(cp->ct_cmd) == FC_FS_ACC) {
   595                  if (fr_len(fp) < sizeof(struct fc_frame_header) +
   596                      sizeof(*cp) + sizeof(*pn))
   597                          goto redisc;
   598                  pn = (struct fc_ns_gid_pn *)(cp + 1);
   599                  port_name = get_unaligned_be64(&pn->fn_wwpn);
   600                  mutex_lock(&rdata->rp_mutex);
   601                  if (rdata->ids.port_name == -1)
   602                          rdata->ids.port_name = port_name;
   603                  else if (rdata->ids.port_name != port_name) {
   604                          FC_DISC_DBG(disc, "GPN_ID accepted.  WWPN changed. "
   605                                      "Port-id %6.6x wwpn %16.16llx\n",
   606                                      rdata->ids.port_id, port_name);
   607                          mutex_unlock(&rdata->rp_mutex);
   608                          fc_rport_logoff(rdata);
   609                          mutex_lock(&lport->disc.disc_mutex);
   610                          new_rdata = fc_rport_create(lport, rdata->ids.port_id);
   611                          mutex_unlock(&lport->disc.disc_mutex);
   612                          if (new_rdata) {
   613                                  new_rdata->disc_id = disc->disc_id;
   614                                  fc_rport_login(new_rdata);
   615                          }
   616                          goto free_fp;
   617                  }
   618                  rdata->disc_id = disc->disc_id;
   619                  mutex_unlock(&rdata->rp_mutex);
   620                  fc_rport_login(rdata);
   621          } else if (ntohs(cp->ct_cmd) == FC_FS_RJT) {
   622                  FC_DISC_DBG(disc, "GPN_ID rejected reason %x exp %x\n",
   623                              cp->ct_reason, cp->ct_explan);
   624                  fc_rport_logoff(rdata);
   625          } else {
   626                  FC_DISC_DBG(disc, "GPN_ID unexpected response code %x\n",
   627                              ntohs(cp->ct_cmd));
   628  redisc:
   629                  mutex_lock(&disc->disc_mutex);
   630                  fc_disc_restart(disc);
   631                  mutex_unlock(&disc->disc_mutex);
   632          }
   633  free_fp:
   634          fc_frame_free(fp);
                ^^^^^^^^^^^^^^^^^
Then this free was added.


   635  out:
   636          kref_put(&rdata->kref, fc_rport_destroy);
   637          if (!IS_ERR(fp))
   638                  fc_frame_free(fp);
                        ^^^^^^^^^^^^^^^^^
But there was already a free here so it was a double free.

   639  }

regards,
dan carpenter