mts-io: clean up memory allocated for accessory cards on module unload or setup failure

4 files changed, 101 insertions, 23 deletions

diff --git a/io-module/mtac_gpiob.c b/io-module/mtac_gpiob.c
index 728634d..f2cd9b2 100644
--- a/io-module/mtac_gpiob.c
+++ b/io-module/mtac_gpiob.c
@@ -395,6 +395,10 @@ static bool gpiob_setup(enum ap port) {
 		return false;
 	}
 
+	// mark the attribute indices we're using so we know what to clean up
+	port_info[port_index]->attrs_start = device_attrs_size;
+	port_info[port_index]->attrs_end = device_attrs_size + ap_gpiob_attrs_size;
+
 	// add digital inputs
 	for (i = 0; i < 4; i++) {
 		sprintf(buf, "din%d:%d", i, port);
@@ -506,18 +510,47 @@ static bool gpiob_setup(enum ap port) {
 }
 
 static bool gpiob_teardown(enum ap port) {
+	int i;
 	int port_index = port - 1;
 
-	// do we need to clean up allocated memory here as well?
 	log_info("unloading GPIOB accessory card in port %d", port);
+
+	// clean up allocated memory for attributes
+	for (i = port_info[port_index]->attrs_start; i < port_info[port_index]->attrs_end; i++) {
+		if (device_attrs[i]) {
+			if (device_attrs[i]->name)
+				kfree(device_attrs[i]->name);
+
+			kfree(device_attrs[i]);
+		}
+	}
+
+	// clean up allocated memory for SPI drivers
+	if (gpiob_spi_drivers[port_index][dout].driver.name)
+		kfree(gpiob_spi_drivers[port_index][dout].driver.name);
+	if (gpiob_spi_drivers[port_index][din].driver.name)
+		kfree(gpiob_spi_drivers[port_index][din].driver.name);
+	if (gpiob_spi_drivers[port_index][adc].driver.name)
+		kfree(gpiob_spi_drivers[port_index][adc].driver.name);
+
+	// unregister SPI drivers
 	spi_unregister_driver(&gpiob_spi_drivers[port_index][dout]);
 	spi_unregister_driver(&gpiob_spi_drivers[port_index][din]);
 	spi_unregister_driver(&gpiob_spi_drivers[port_index][adc]);
+
+	// reset attribute index markers
+	port_info[port_index]->attrs_start = 0;
+	port_info[port_index]->attrs_end = 0;
+
 	return true;
 }
 
-static struct ap_info gpiob_info = {
-	.product_id = MTAC_GPIOB_0_0,
-	.setup = &gpiob_setup,
-	.teardown = &gpiob_teardown
-};
+bool set_gpiob_info(struct ap_info* info) {
+	info->product_id = MTAC_GPIOB_0_0;
+	info->setup = &gpiob_setup;
+	info->teardown = &gpiob_teardown;
+	info->attrs_start = 0;
+	info->attrs_end = 0;
+
+	return true;
+}
diff --git a/io-module/mtac_mfser.c b/io-module/mtac_mfser.c
index 5871bfe..207180f 100644
--- a/io-module/mtac_mfser.c
+++ b/io-module/mtac_mfser.c
@@ -190,6 +190,7 @@ static ssize_t mts_attr_store_mfser_mode(struct kobject *kobj,
 static int ap_mfser_attrs_size = 3;
 
 static bool mfser_setup(enum ap port) {
+	int port_index = port - 1;
 	struct kobj_attribute *attr;
 	char buf[32];
 
@@ -200,6 +201,10 @@ static bool mfser_setup(enum ap port) {
 		return false;
 	}
 
+	// mark the attribute indices we're using so we know what to clean up
+	port_info[port_index]->attrs_start = device_attrs_size;
+	port_info[port_index]->attrs_end = device_attrs_size + ap_mfser_attrs_size;
+
 	sprintf(buf, "serial-mode:%d", port);
 	attr = create_attribute(buf, MTS_ATTR_MODE_RW);
 	if (! attr) {
@@ -234,12 +239,33 @@ static bool mfser_setup(enum ap port) {
 }
 
 static bool mfser_teardown(enum ap port) {
+	int i;
+	int port_index = port - 1;
+
 	log_info("unloading MFSER accessory card in port %d", port);
+
+	// clean up allocated memory for attributes
+	for (i = port_info[port_index]->attrs_start; i < port_info[port_index]->attrs_end; i++) {
+		if (device_attrs[i]) {
+			if (device_attrs[i]->name)
+				kfree(device_attrs[i]->name);
+
+			kfree(device_attrs[i]);
+		}
+	}
+
+	port_info[port_index]->attrs_start = 0;
+	port_info[port_index]->attrs_end = 0;
+
 	return true;
 }
 
-static struct ap_info mfser_info = {
-	.product_id = MTAC_MFSER_0_0,
-	.setup = &mfser_setup,
-	.teardown = &mfser_teardown
-};
+bool set_mfser_info(struct ap_info* info) {
+	info->product_id = MTAC_MFSER_0_0;
+	info->setup = &mfser_setup;
+	info->teardown = &mfser_teardown;
+	info->attrs_start = 0;
+	info->attrs_end = 0;
+
+	return true;
+}
diff --git a/io-module/mts_io.c b/io-module/mts_io.c
index 936d0d2..8b982c6 100644
--- a/io-module/mts_io.c
+++ b/io-module/mts_io.c
@@ -434,10 +434,22 @@ static bool load_port(int port) {
 	} else if (mts_ap_eeprom[port_index][0] == 0x00) {
 		log_info("no accessory card inserted in port %d", port);
 	} else {
+		port_info[port_index] = kzalloc(sizeof(struct ap_info), GFP_KERNEL);
+		if (! port_info[port_index]) {
+			log_error("alloc of port info failed");
+			return false;
+		}
+
 		if (strstr(ap_eeprom[port_index].product_id, PRODUCT_ID_MTAC_GPIOB)) {
-			port_info[port_index] = &gpiob_info;
+			if (! set_gpiob_info(port_info[port_index])) {
+				log_error("failed to set up gpiob port info");
+				return false;
+			}
 		} else if (strstr(ap_eeprom[port_index].product_id, PRODUCT_ID_MTAC_MFSER)) {
-			port_info[port_index] = &mfser_info;
+			if (! set_mfser_info(port_info[port_index])) {
+				log_error("failed to set up mfser port info");
+				return false;
+			}
 		} else {
 			log_error("unknown accessory card [%s] in port %d", ap_eeprom[port_index].product_id, port);
 			return false;
@@ -461,6 +473,7 @@ static bool load_port(int port) {
 		if (! port_info[port_index]->setup(port)) {
 			log_error("accessory port %d setup failed", port);
 			port_info[port_index]->teardown(port);
+			kfree(port_info[port]);
 			return false;
 		}
 	}
@@ -645,7 +658,8 @@ static int __init mts_io_init(void)
 	int ret;
 	size_t device_attributes_size;
 	size_t card_attributes_size;
-	int i;
+	int port;
+	int port_index;
 
 	log_info("init: " DRIVER_VERSION);
 	
@@ -655,8 +669,8 @@ static int __init mts_io_init(void)
 	}
 
 	if (NUM_AP) {
-		for (i = 0; i < NUM_AP; i++) {
-			port_info[i] = NULL;
+		for (port = 0; port < NUM_AP; port++) {
+			port_info[port] = NULL;
 		}
 		init_accessory_ports();
 	}
@@ -785,9 +799,10 @@ error2:
 	platform_device_put(mts_io_platform_device);
 error1:
 	log_error("init failed: %d", ret);
-	for (i = 0; i < NUM_AP; i++) {
-		if (port_info[i]) {
-			port_info[i]->teardown(i);
+	for (port = 0, port_index = 1; port < NUM_AP; port++, port_index++) {
+		if (port_info[port]) {
+			port_info[port]->teardown(port_index);
+			kfree(port_info[port]);
 		}
 	}
 
@@ -796,7 +811,8 @@ error1:
 
 static void __exit mts_io_exit(void)
 {
-	int i;
+	int port;
+	int port_index;
 
 	if ( mts_product_id != MT100EOCG_0_0 ) {
 		cancel_delayed_work_sync(&reset_work);
@@ -824,9 +840,10 @@ static void __exit mts_io_exit(void)
 
 	platform_device_unregister(mts_io_platform_device);
 
-	for (i = 0; i < NUM_AP; i++) {
-		if (port_info[i]) {
-			port_info[i]->teardown(i);
+	for (port = 0, port_index = 1; port < NUM_AP; port++, port_index++) {
+		if (port_info[port]) {
+			port_info[port]->teardown(port_index);
+			kfree(port_info[port]);
 		}
 	}
 
diff --git a/io-module/mts_io.h b/io-module/mts_io.h
index ac39463..2aedd3e 100644
--- a/io-module/mts_io.h
+++ b/io-module/mts_io.h
@@ -120,6 +120,8 @@ struct ap_info {
 	uint8_t product_id;
 	bool (*setup)(enum ap port);
 	bool (*teardown)(enum ap port);
+	int attrs_start;
+	int attrs_end;
 };
 
 #endif /* ~__MTS_IO_H */