Re: [libvirt] [PATCH] phyp: Fixing possible buffer overflow
On 07/15/2010 11:01 PM, Laine Stump wrote:
On 07/15/2010 06:37 PM, Eduardo Otubo wrote:
> The line src/phyp/phyp_driver.c:427 was crashing by buffer overflow
> if the return of the command wasn't<=10. The highest number for a
> LPAR ID is 256 per machine, no need to allocate 10 bytes for it. So,
> adjusting the correct size (+1 byte for the '\n') and checking for
> errors.
This analysis doesn't make sense to me. On the one hand, you say that
the function was crashing if the return from the command wasn't<= 10
bytes long (implying that it sometimes was longer), on the other hand,
you say that part of the solution is to change the code to only handle
4 bytes (you forgot about the trailing NULL there, BTW - should have
been 3), because you'll never get more than that.
I'm sorry. My bad here. The lssyscfg (with the correct parameters and
greps) will return something like this:
5
4
6
7
10
1
143
These are the LPAR IDs. Each line cannot be bigger than 3 characters (+1
for '\n'). So, I just allocate 4 bytes to hold these values, one at a
time and memset it with zeros. If there's an error, there will be a
single line with the error message, like this one:
"HSCL8018 The managed system rico was not found."
IMO the best choice is to loop in the result and if I can't find a '\n'
after 3 character, I return an error. My mistake was to check the entire
string returned from exec. My bad, sorry.
I think this patch is much clearer on what the problem is.
Thanks for the comments!
---
src/phyp/phyp_driver.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index ee1e21b..0c50c9a 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -383,7 +383,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids,
int nids,
int got = 0;
char *char_ptr;
unsigned int i = 0, j = 0;
- char id_c[10];
+ char id_c[4];
char *cmd = NULL;
char *ret = NULL;
const char *state;
@@ -394,7 +394,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids,
int nids,
else
state = " ";
- memset(id_c, 0, 10);
+ memset(id_c, 0, 4);
virBufferAddLit(&buf, "lssyscfg -r lpar");
if (system_type == HMC)
@@ -414,7 +414,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids,
int nids,
if (exit_status < 0 || ret == NULL)
goto err;
else {
- while (got < nids) {
+ while (got < nids && i <= 4) {
if (ret[i] == '\0')
break;
else if (ret[i] == '\n') {
--
1.7.0.4