Commit fc2b57c9af46 ("xenstored: send an evtchn notification on
introduce_domain") introduced a potential NULL dereference in case of
Xenstore live update.

Fix that by adding an appropriate check.

Coverity-Id: 1504572
Fixes: fc2b57c9af46 ("xenstored: send an evtchn notification on introduce_domain")
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_domain.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index de88bf2a68..ead4c237d2 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -493,9 +493,11 @@ static struct domain *introduce_domain(const void *ctx,
 		/* Now domain belongs to its connection. */
 		talloc_steal(domain->conn, domain);
 
-		/* Notify the domain that xenstore is available */
-		interface->connection = XENSTORE_CONNECTED;
-		xenevtchn_notify(xce_handle, domain->port);
+		if (!restore) {
+			/* Notify the domain that xenstore is available */
+			interface->connection = XENSTORE_CONNECTED;
+			xenevtchn_notify(xce_handle, domain->port);
+		}
 
 		if (!is_master_domain && !restore)
 			fire_watches(NULL, ctx, "@introduceDomain", NULL,
-- 
2.35.3

On 25/05/2022 11:55, Juergen Gross wrote:
> Commit fc2b57c9af46 ("xenstored: send an evtchn notification on
> introduce_domain") introduced a potential NULL dereference in case of
> Xenstore live update.
>
> Fix that by adding an appropriate check.
>
> Coverity-Id: 1504572
> Fixes: fc2b57c9af46 ("xenstored: send an evtchn notification on introduce_domain")
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> seeing as I've
just looked at the Coverity report too.

CC'ing the others involved in the original patch just so they're aware
it was broken.

~Andrew

> ---
>  tools/xenstore/xenstored_domain.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
> index de88bf2a68..ead4c237d2 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -493,9 +493,11 @@ static struct domain *introduce_domain(const void *ctx,
>  		/* Now domain belongs to its connection. */
>  		talloc_steal(domain->conn, domain);
>  
> -		/* Notify the domain that xenstore is available */
> -		interface->connection = XENSTORE_CONNECTED;
> -		xenevtchn_notify(xce_handle, domain->port);
> +		if (!restore) {
> +			/* Notify the domain that xenstore is available */
> +			interface->connection = XENSTORE_CONNECTED;
> +			xenevtchn_notify(xce_handle, domain->port);
> +		}
>  
>  		if (!is_master_domain && !restore)
>  			fire_watches(NULL, ctx, "@introduceDomain", NULL,

Hi Andrew,

On 25/05/2022 11:59, Andrew Cooper wrote:
> On 25/05/2022 11:55, Juergen Gross wrote:
>> Commit fc2b57c9af46 ("xenstored: send an evtchn notification on
>> introduce_domain") introduced a potential NULL dereference in case of
>> Xenstore live update.
>>
>> Fix that by adding an appropriate check.
>>
>> Coverity-Id: 1504572
>> Fixes: fc2b57c9af46 ("xenstored: send an evtchn notification on introduce_domain")
>> Signed-off-by: Juergen Gross <jgross@suse.com>

Committed.

> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> seeing as I've

b4 will end up to pick "seeing as I've":

42sh> b4 am 21392cc6-55b6-647e-08eb-c818d6229603@srcf.net
Looking up 
https://lore.kernel.org/r/21392cc6-55b6-647e-08eb-c818d6229603%40srcf.net
Grabbing thread from 
lore.kernel.org/all/21392cc6-55b6-647e-08eb-c818d6229603%40srcf.net/t.mbox.gz
Analyzing 2 messages in the thread
Checking attestation on all messages, may take a moment...
---
   ✓ [PATCH] tools/xenstore: fix event sending in introduce_domain()
     + Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> seeing as I've
   ---
   ✓ Signed: DKIM/suse.com
---
Total patches: 1
---
  Link: https://lore.kernel.org/r/20220525105549.30184-1-jgross@suse.com
  Base: applies clean to current tree
        git am 
./20220525_jgross_tools_xenstore_fix_event_sending_in_introduce_domain.mbx

I don't think this is fixable in b4 because we allow to have additional 
information after the tag (e.g. # arm).

So would you be able to avoid adding words after the tags that are not 
meant to committed? This would reduce the amount of manual work when 
committing.

Cheers,

-- 
Julien Grall