11:48 a.m.
Copying the pointer caused double-free when clearing the domain object.
Peter Krempa (2):
virSecretLookupDefCopy: Remove return value
virStorageEncryptionSecretCopy: Properly copy internals
src/util/virsecret.c | 3 +--
src/util/virsecret.h | 4 ++--
src/util/virstorageencryption.c | 8 +++-----
src/util/virstoragefile.c | 3 +--
4 files changed, 7 insertions(+), 11 deletions(-)
--
2.24.1
11:48 a.m.
New subject: [PATCH 1/2] virSecretLookupDefCopy: Remove return value
The function always returns succes so there's no need for a return
value.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virsecret.c | 3 +--
src/util/virsecret.h | 4 ++--
src/util/virstoragefile.c | 3 +--
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/util/virsecret.c b/src/util/virsecret.c
index f44d964198..54d6bbcb7c 100644
--- a/src/util/virsecret.c
+++ b/src/util/virsecret.c
@@ -48,7 +48,7 @@ virSecretLookupDefClear(virSecretLookupTypeDefPtr def)
}
-int
+void
virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst,
const virSecretLookupTypeDef *src)
{
@@ -58,7 +58,6 @@ virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst,
} else if (dst->type == VIR_SECRET_LOOKUP_TYPE_USAGE) {
dst->u.usage = g_strdup(src->u.usage);
}
- return 0;
}
diff --git a/src/util/virsecret.h b/src/util/virsecret.h
index 8c49cfbc89..cfdf2b6e29 100644
--- a/src/util/virsecret.h
+++ b/src/util/virsecret.h
@@ -48,8 +48,8 @@ struct _virSecretLookupTypeDef {
};
void virSecretLookupDefClear(virSecretLookupTypeDefPtr def);
-int virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst,
- const virSecretLookupTypeDef *src);
+void virSecretLookupDefCopy(virSecretLookupTypeDefPtr dst,
+ const virSecretLookupTypeDef *src);
int virSecretLookupParseSecret(xmlNodePtr secretnode,
virSecretLookupTypeDefPtr def);
void virSecretLookupFormatSecret(virBufferPtr buf,
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 870c40f446..e723cc9410 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1792,8 +1792,7 @@ virStorageAuthDefCopy(const virStorageAuthDef *src)
authdef->secrettype = g_strdup(src->secrettype);
authdef->authType = src->authType;
- if (virSecretLookupDefCopy(&authdef->seclookupdef, &src->seclookupdef)
< 0)
- return NULL;
+ virSecretLookupDefCopy(&authdef->seclookupdef, &src->seclookupdef);
return g_steal_pointer(&authdef);
}
--
2.24.1
11:48 a.m.
New subject: [PATCH 2/2] virStorageEncryptionSecretCopy: Properly copy internals
virStorageEncryptionSecretPtr may have a string inside it, thus we must
copy the string too. Use virSecretLookupDefCopy to do that.
Likely caused by 756b46ddd24.
https://bugzilla.redhat.com/show_bug.cgi?id=1814923
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virstorageencryption.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c
index 74836d4a00..6765fdc23a 100644
--- a/src/util/virstorageencryption.c
+++ b/src/util/virstorageencryption.c
@@ -85,12 +85,10 @@ virStorageEncryptionFree(virStorageEncryptionPtr enc)
static virStorageEncryptionSecretPtr
virStorageEncryptionSecretCopy(const virStorageEncryptionSecret *src)
{
- virStorageEncryptionSecretPtr ret;
-
- if (VIR_ALLOC(ret) < 0)
- return NULL;
+ virStorageEncryptionSecretPtr ret = g_new0(virStorageEncryptionSecret, 1);
- memcpy(ret, src, sizeof(*src));
+ ret->type = src->type;
+ virSecretLookupDefCopy(&ret->seclookupdef, &src->seclookupdef);
return ret;
}
--
2.24.1
12:14 p.m.
New subject: [PATCH 2/2] virStorageEncryptionSecretCopy: Properly copy internals
On a Thursday in 2020, Peter Krempa wrote:
virStorageEncryptionSecretPtr may have a string inside it, thus we
must
copy the string too. Use virSecretLookupDefCopy to do that.
Likely caused by 756b46ddd24.
Please remove the period from the end.
Also, at the times of that commit, memcpy was sufficient:
struct _virStorageEncryptionSecret {
int type; /* virStorageEncryptionSecretType */
unsigned char uuid[VIR_UUID_BUFLEN];
};
So this actually fixes commit 47e88b33b which switched to using
virSecretLookupDef.
https://bugzilla.redhat.com/show_bug.cgi?id=1814923
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virstorageencryption.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
12:24 p.m.
New subject: [PATCH 2/2] virStorageEncryptionSecretCopy: Properly copy internals
On Thu, Mar 19, 2020 at 18:14:15 +0100, Ján Tomko wrote:
On a Thursday in 2020, Peter Krempa wrote:
> virStorageEncryptionSecretPtr may have a string inside it, thus we must
> copy the string too. Use virSecretLookupDefCopy to do that.
>
> Likely caused by 756b46ddd24.
Please remove the period from the end.
Also, at the times of that commit, memcpy was sufficient:
struct _virStorageEncryptionSecret {
int type; /* virStorageEncryptionSecretType */
unsigned char uuid[VIR_UUID_BUFLEN];
};
So this actually fixes commit 47e88b33b which switched to using
virSecretLookupDef.
Oh, okay, I didn't look as closely into the history. I'll update the
problematic commit.
12:43 p.m.
New subject: [PATCH 2/2] virStorageEncryptionSecretCopy: Properly copy internals
On a Thursday in 2020, Peter Krempa wrote:
On Thu, Mar 19, 2020 at 18:14:15 +0100, Ján Tomko wrote:
> On a Thursday in 2020, Peter Krempa wrote:
> > virStorageEncryptionSecretPtr may have a string inside it, thus we must
> > copy the string too. Use virSecretLookupDefCopy to do that.
> >
> > Likely caused by 756b46ddd24.
>
> Please remove the period from the end.
>
> Also, at the times of that commit, memcpy was sufficient:
> struct _virStorageEncryptionSecret {
> int type; /* virStorageEncryptionSecretType */
> unsigned char uuid[VIR_UUID_BUFLEN];
> };
>
> So this actually fixes commit 47e88b33b which switched to using
> virSecretLookupDef.
Oh, okay, I didn't look as closely into the history. I'll update the
problematic commit.
Well, 756b46ddd24 is also problematic - something like this is easy to
overlook.
Jano
12:14 p.m.
On a Thursday in 2020, Peter Krempa wrote:
Copying the pointer caused double-free when clearing the domain
object.
Peter Krempa (2):
virSecretLookupDefCopy: Remove return value
virStorageEncryptionSecretCopy: Properly copy internals
src/util/virsecret.c | 3 +--
src/util/virsecret.h | 4 ++--
src/util/virstorageencryption.c | 8 +++-----
src/util/virstoragefile.c | 3 +--
4 files changed, 7 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
12:18 p.m.
On 19. 3. 2020 17:48, Peter Krempa wrote:
Copying the pointer caused double-free when clearing the domain
object.
Peter Krempa (2):
virSecretLookupDefCopy: Remove return value
virStorageEncryptionSecretCopy: Properly copy internals
src/util/virsecret.c | 3 +--
src/util/virsecret.h | 4 ++--
src/util/virstorageencryption.c | 8 +++-----
src/util/virstoragefile.c | 3 +--
4 files changed, 7 insertions(+), 11 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1709
days inactive
1709
days old
7 comments
3 participants
participants (3)
-
Ján Tomko -
Michal Prívozník -
Peter Krempa