* [PATCH 2/2] xenstat: don't leak memory in getBridge
2014-05-04 8:31 [PATCH 1/2] xenstat: fix unsigned less-than-0 comparison Matthew Daley
@ 2014-05-04 8:31 ` Matthew Daley
2014-05-05 14:32 ` [PATCH 1/2] xenstat: fix unsigned less-than-0 comparison Konrad Rzeszutek Wilk
1 sibling, 0 replies; 4+ messages in thread
From: Matthew Daley @ 2014-05-04 8:31 UTC (permalink / raw)
To: xen-devel
Cc: xen-devel, Matthew Daley, Ian Jackson, Ian Campbell,
Stefano Stabellini
getBridge's method of returning a result was a little confused:
allocating a result buffer but never using it.
Simplify by instead allowing a result buffer to be passed in and
modifying the single usage to match.
Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
---
tools/xenstat/libxenstat/src/xenstat_linux.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/tools/xenstat/libxenstat/src/xenstat_linux.c b/tools/xenstat/libxenstat/src/xenstat_linux.c
index 931b24e..24335e1 100644
--- a/tools/xenstat/libxenstat/src/xenstat_linux.c
+++ b/tools/xenstat/libxenstat/src/xenstat_linux.c
@@ -64,14 +64,12 @@ static const char PROCNETDEV_HEADER[] =
/* We need to get the name of the bridge interface for use with bonding interfaces */
/* Use excludeName parameter to avoid adding bridges we don't care about, eg. virbr0 */
-char *getBridge(char *excludeName)
+void getBridge(char *excludeName, char *result, size_t resultLen)
{
struct dirent *de;
DIR *d;
- char tmp[256] = { 0 }, *bridge;
-
- bridge = (char *)malloc(16 * sizeof(char));
+ char tmp[256] = { 0 };
d = opendir("/sys/class/net");
while ((de = readdir(d)) != NULL) {
@@ -79,14 +77,14 @@ char *getBridge(char *excludeName)
&& (strstr(de->d_name, excludeName) == NULL)) {
sprintf(tmp, "/sys/class/net/%s/bridge", de->d_name);
- if (access(tmp, F_OK) == 0)
- bridge = de->d_name;
+ if (access(tmp, F_OK) == 0) {
+ strncpy(result, de->d_name, resultLen - 1);
+ result[resultLen - 1] = 0;
+ }
}
}
closedir(d);
-
- return bridge;
}
/* parseNetLine provides regular expression based parsing for lines from /proc/net/dev, all the */
@@ -279,7 +277,7 @@ int xenstat_collect_networks(xenstat_node * node)
SEEK_SET);
/* We get the bridge devices for use with bonding interface to get bonding interface stats */
- snprintf(devBridge, 16, "%s", getBridge("vir"));
+ getBridge("vir", devBridge, sizeof(devBridge));
snprintf(devNoBridge, 16, "p%s", devBridge);
while (fgets(line, 512, priv->procnetdev)) {
--
1.9.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] xenstat: fix unsigned less-than-0 comparison
2014-05-04 8:31 [PATCH 1/2] xenstat: fix unsigned less-than-0 comparison Matthew Daley
2014-05-04 8:31 ` [PATCH 2/2] xenstat: don't leak memory in getBridge Matthew Daley
@ 2014-05-05 14:32 ` Konrad Rzeszutek Wilk
2014-05-08 10:39 ` Ian Campbell
1 sibling, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-05-05 14:32 UTC (permalink / raw)
To: Matthew Daley
Cc: xen-devel, Stefano Stabellini, Ian Jackson, Ian Campbell,
xen-devel
On Sun, May 04, 2014 at 08:31:46PM +1200, Matthew Daley wrote:
> Commit 1438d36f ("xenstat: Fix buffer over-run with new_domains being
> negative.") attempted to fix the handling of a negative error result
> from xc_domain_getinfolist in xenstat_get_node. However, it forgot to
> change the result variable from an unsigned type to a signed one.
>
> Do so, allowing the error result to be handled properly.
>
> Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
I was looking at this by changing the underlay libxc calls to
using 'unsigned' instead of 'int' but that has kind of been
preempted.
Thanks for fixing it!
> ---
> tools/xenstat/libxenstat/src/xenstat.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/xenstat/libxenstat/src/xenstat.c b/tools/xenstat/libxenstat/src/xenstat.c
> index e5facb8..8072a90 100644
> --- a/tools/xenstat/libxenstat/src/xenstat.c
> +++ b/tools/xenstat/libxenstat/src/xenstat.c
> @@ -164,7 +164,7 @@ xenstat_node *xenstat_get_node(xenstat_handle * handle, unsigned int flags)
> xenstat_node *node;
> xc_physinfo_t physinfo = { 0 };
> xc_domaininfo_t domaininfo[DOMAIN_CHUNK_SIZE];
> - unsigned int new_domains;
> + int new_domains;
> unsigned int i;
>
> /* Create the node */
> --
> 1.9.2
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread