Skip to content

Commit

Permalink
ata,scsi: libata-core: Do not leak memory for ata_port struct members
Browse files Browse the repository at this point in the history
libsas is currently not freeing all the struct ata_port struct members,
e.g. ncq_sense_buf for a driver supporting Command Duration Limits (CDL).

Add a function, ata_port_free(), that is used to free a ata_port,
including its struct members. It makes sense to keep the code related to
freeing a ata_port in its own function, which will also free all the
struct members of struct ata_port.

Fixes: 18bd771 ("scsi: ata: libata: Handle completion of CDL commands using policy 0xD")
Reviewed-by: John Garry <john.g.garry@oracle.com>
Link: https://lore.kernel.org/r/20240629124210.181537-8-cassel@kernel.org
Signed-off-by: Niklas Cassel <cassel@kernel.org>
  • Loading branch information
Niklas Cassel committed Jun 30, 2024
1 parent 5d92c7c commit f6549f5
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 14 deletions.
24 changes: 14 additions & 10 deletions drivers/ata/libata-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -5489,6 +5489,18 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
return ap;
}

void ata_port_free(struct ata_port *ap)
{
if (!ap)
return;

kfree(ap->pmp_link);
kfree(ap->slave_link);
kfree(ap->ncq_sense_buf);
kfree(ap);
}
EXPORT_SYMBOL_GPL(ata_port_free);

static void ata_devres_release(struct device *gendev, void *res)
{
struct ata_host *host = dev_get_drvdata(gendev);
Expand All @@ -5515,15 +5527,7 @@ static void ata_host_release(struct kref *kref)
int i;

for (i = 0; i < host->n_ports; i++) {
struct ata_port *ap = host->ports[i];

if (!ap)
continue;

kfree(ap->pmp_link);
kfree(ap->slave_link);
kfree(ap->ncq_sense_buf);
kfree(ap);
ata_port_free(host->ports[i]);
host->ports[i] = NULL;
}
kfree(host);
Expand Down Expand Up @@ -5906,7 +5910,7 @@ int ata_host_register(struct ata_host *host, const struct scsi_host_template *sh
* allocation time.
*/
for (i = host->n_ports; host->ports[i]; i++)
kfree(host->ports[i]);
ata_port_free(host->ports[i]);

/* give ports names and add SCSI hosts */
for (i = 0; i < host->n_ports; i++) {
Expand Down
6 changes: 3 additions & 3 deletions drivers/scsi/libsas/sas_ata.c
Original file line number Diff line number Diff line change
Expand Up @@ -610,15 +610,15 @@ int sas_ata_init(struct domain_device *found_dev)

rc = ata_sas_tport_add(ata_host->dev, ap);
if (rc)
goto destroy_port;
goto free_port;

found_dev->sata_dev.ata_host = ata_host;
found_dev->sata_dev.ap = ap;

return 0;

destroy_port:
kfree(ap);
free_port:
ata_port_free(ap);
free_host:
ata_host_put(ata_host);
return rc;
Expand Down
2 changes: 1 addition & 1 deletion drivers/scsi/libsas/sas_discover.c
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ void sas_free_device(struct kref *kref)

if (dev_is_sata(dev) && dev->sata_dev.ap) {
ata_sas_tport_delete(dev->sata_dev.ap);
kfree(dev->sata_dev.ap);
ata_port_free(dev->sata_dev.ap);
ata_host_put(dev->sata_dev.ata_host);
dev->sata_dev.ata_host = NULL;
dev->sata_dev.ap = NULL;
Expand Down
1 change: 1 addition & 0 deletions include/linux/libata.h
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,7 @@ extern int ata_slave_link_init(struct ata_port *ap);
extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
struct ata_port_info *, struct Scsi_Host *);
extern void ata_port_probe(struct ata_port *ap);
extern void ata_port_free(struct ata_port *ap);
extern int ata_sas_tport_add(struct device *parent, struct ata_port *ap);
extern void ata_sas_tport_delete(struct ata_port *ap);
int ata_sas_device_configure(struct scsi_device *sdev, struct queue_limits *lim,
Expand Down

0 comments on commit f6549f5

Please sign in to comment.