Skip to contentSkip to conversation

Missing error checking in cmirrord

PATCH New Jonathan Brassow 3 participants 3 messages Jun 18, 2010 – Jun 21, 2010

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Missing error checking in cmirrord

There are missing error checks... it's possible for sprintf to fail.

brassow

daemons/cmirrord/functions.c +35 -15
Index: LVM2/daemons/cmirrord/functions.c
===================================================================
@@ -1371,15 +1371,21 @@ static int clog_get_sync_count(struct dm
 
 static int core_status_info(struct log_c *lc __attribute((unused)), struct dm_ulog_request *rq)
 {
+	int r;
 	char *data = (char *)rq->data;
 
-	rq->data_size = sprintf(data, "1 clustered-core");
+	r = sprintf(data, "1 clustered-core");
+	if (r < 0)
+		return r;
+
+	rq->data_size = r;
 
 	return 0;
 }
 
 static int disk_status_info(struct log_c *lc, struct dm_ulog_request *rq)
 {
+	int r;
 	char *data = (char *)rq->data;
 	struct stat statbuf;
 
@@ -1388,9 +1394,13 @@ static int disk_status_info(struct log_c
 		return -errno;
 	}
 
-	rq->data_size = sprintf(data, "3 clustered-disk %d:%d %c",
-				major(statbuf.st_rdev), minor(statbuf.st_rdev),
-				(lc->log_dev_failed) ? 'D' : 'A');
+	r = sprintf(data, "3 clustered-disk %d:%d %c",
+		    major(statbuf.st_rdev), minor(statbuf.st_rdev),
+		    (lc->log_dev_failed) ? 'D' : 'A');
+	if (r < 0)
+		return r;
+
+	rq->data_size = r;
 
 	return 0;
 }
@@ -1421,18 +1431,24 @@ static int clog_status_info(struct dm_ul
 
 static int core_status_table(struct log_c *lc, struct dm_ulog_request *rq)
 {
+	int r;
 	char *data = (char *)rq->data;
 
-	rq->data_size = sprintf(data, "clustered-core %u %s%s ",
-				lc->region_size,
-				(lc->sync == DEFAULTSYNC) ? "" :
-				(lc->sync == NOSYNC) ? "nosync " : "sync ",
-				(lc->block_on_error) ? "block_on_error" : "");
+	r = sprintf(data, "clustered-core %u %s%s ",
+		    lc->region_size,
+		    (lc->sync == DEFAULTSYNC) ? "" :
+		    (lc->sync == NOSYNC) ? "nosync " : "sync ",
+		    (lc->block_on_error) ? "block_on_error" : "");
+	if (r < 0)
+		return r;
+
+	rq->data_size = r;
 	return 0;
 }
 
 static int disk_status_table(struct log_c *lc, struct dm_ulog_request *rq)
 {
+	int r;
 	char *data = (char *)rq->data;
 	struct stat statbuf;
 
@@ -1441,12 +1457,16 @@ static int disk_status_table(struct log_
 		return -errno;
 	}
 
-	rq->data_size = sprintf(data, "clustered-disk %d:%d %u %s%s ",
-				major(statbuf.st_rdev), minor(statbuf.st_rdev),
-				lc->region_size,
-				(lc->sync == DEFAULTSYNC) ? "" :
-				(lc->sync == NOSYNC) ? "nosync " : "sync ",
-				(lc->block_on_error) ? "block_on_error" : "");
+	r = sprintf(data, "clustered-disk %d:%d %u %s%s ",
+		    major(statbuf.st_rdev), minor(statbuf.st_rdev),
+		    lc->region_size,
+		    (lc->sync == DEFAULTSYNC) ? "" :
+		    (lc->sync == NOSYNC) ? "nosync " : "sync ",
+		    (lc->block_on_error) ? "block_on_error" : "");
+	if (r < 0)
+		return r;
+
+	rq->data_size = r;
 	return 0;
 }
 
#1 lore raw

PATCH 1/1 Missing error checking in cmirrord

Quoted 2 lines · Jon Brassow

There are missing error checks... it's possible for sprintf to fail.

And an inline comment to explain how data has an adequate minimum size perhaps?

Alasdair

#2 lore raw

PATCH 1/1 Missing error checking in cmirrord

In reply to Jonathan Brassow
Quoted 10 lines · Jonathan Brassow

There are missing error checks... it's possible for sprintf to fail.

brassow

Index: LVM2/daemons/cmirrord/functions.c
===================================================================

daemons/cmirrord/functions.c static int clog_get_sync_count(struct dm · line 1376

Quoted 7 lines
daemons/cmirrord/functions.c +1 -1
@@ -1371,15 +1371,21 @@ static int clog_get_sync_count(struct dm
 
 static int core_status_info(struct log_c *lc __attribute((unused)), struct dm_ulog_request *rq)
 {
+	int r;
 	char *data = (char *)rq->data;
 
-	rq->data_size = sprintf(data, "1 clustered-core");
	rq->data_size = 0;

daemons/cmirrord/functions.c static int clog_get_sync_count(struct dm · line 1381

Quoted 5 lines
daemons/cmirrord/functions.c +5 -0
+	r = sprintf(data, "1 clustered-core");
+	if (r < 0)
+		return r;
+
+	rq->data_size = r;
	rq->data_size = r + 1;

sprintf does not count trailing \0 ... (and this is probably part of the mysterious fails of cmirrord).

Milan