Fix off-by-one bug limiting VNC passwords to 7 chars

PATCH Superseded Chris Webb 3 participants 9 messages Nov 23, 2008 – Dec 10, 2008 KVM

lore

Share & tools

lore

Chris Webb

author

2008-11-23 11:31 UTC #0 lore raw

PATCH 1/1 Fix off-by-one bug limiting VNC passwords to 7 chars

Fix off-by-one bug limiting VNC passwords to 7 characters instead of 8

monitor_readline expects buf_size to include the terminating \0, but do_change_vnc in monitor.c calls it as though it doesn't. The other site where monitor_readline reads a password (in vl.c) passes the buffer length correctly.

Signed-off-by Chris Webb <[email protected]>

---
 monitor.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

monitor.c +1 -1

index 22360fc..6ae5729 100644
@@ -433,7 +433,7 @@ static void do_change_vnc(const char *target)
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
 	char password[9];
-	monitor_readline("Password: ", 1, password, sizeof(password)-1);
+	monitor_readline("Password: ", 1, password, sizeof(password));
 	password[sizeof(password)-1] = '\0';
 	if (vnc_display_password(NULL, password) < 0)
 	    term_printf("could not set VNC server password\n");

Thiemo Seufer

2008-11-23 12:31 UTC #1 lore raw

Quoted 17 lines · Chris Webb

Fix off-by-one bug limiting VNC passwords to 7 characters instead of 8
monitor_readline expects buf_size to include the terminating \0, but do_change_vnc in monitor.c calls it as though it doesn't. The other site where monitor_readline reads a password (in vl.c) passes the buffer length correctly.
Signed-off-by Chris Webb <[email protected]>
---
 monitor.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
monitor.c +0 -0
index 22360fc..6ae5729 100644

monitor.c static void do_change_vnc(const char *target) · line 437

Quoted 6 lines

monitor.c +1 -1

@@ -433,7 +433,7 @@ static void do_change_vnc(const char *target)
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
 	char password[9];
-	monitor_readline("Password: ", 1, password, sizeof(password)-1);
+	monitor_readline("Password: ", 1, password, sizeof(password));
 	password[sizeof(password)-1] = '\0';

The next line can go as well, the string is already NULL terminated.

Thiemo

Chris Webb

author

2008-11-25 10:09 UTC #2 lore raw

Quoted 1 line · Thiemo Seufer

Chris Webb wrote:

[...]

Quoted 5 lines

monitor.c static void do_change_vnc(const char *target) · line 437
Quoted 3 lines
monitor.c +1 -1
-	monitor_readline("Password: ", 1, password, sizeof(password)-1);
+	monitor_readline("Password: ", 1, password, sizeof(password));
 	password[sizeof(password)-1] = '\0';
The next line can go as well, the string is already NULL terminated.

You're quite right. I'll update the two patches to reflect this change.

Cheers,

Chris.

Chris Webb

author

2008-11-25 10:25 UTC #3 lore raw

PATCH v2 1/1 Fix off-by-one bug limiting VNC passwords to 7 chars

Fix off-by-one bug limiting VNC passwords to 7 characters instead of 8

Signed-off-by Chris Webb <[email protected]>

---
 monitor.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

monitor.c +1 -2

index 22360fc..a252838 100644
@@ -433,8 +433,7 @@ static void do_change_vnc(const char *target)
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
 	char password[9];
-	monitor_readline("Password: ", 1, password, sizeof(password)-1);
-	password[sizeof(password)-1] = '\0';
+	monitor_readline("Password: ", 1, password, sizeof(password));
 	if (vnc_display_password(NULL, password) < 0)
 	    term_printf("could not set VNC server password\n");
     } else {

Chris Webb

author

2008-11-25 10:26 UTC #4 lore raw

PATCH v2 1/1 Accept password as an argument to 'change vnc password'

Accept password as an argument to 'change vnc password' monitor command

This allows easier use of the change vnc password monitor command from management scripts, without having to implement expect(1)-like behaviour.

Signed-off-by Chris Webb <[email protected]>

---
 monitor.c     |   14 +++++++++-----
 qemu-doc.texi |    8 ++++----
 2 files changed, 13 insertions(+), 9 deletions(-)

monitor.c +9 -5

index a252838..f6a2783 100644
@@ -428,12 +428,16 @@ static void do_change_block(const char *device, const char *filename, const char
     qemu_key_check(bs, filename);
 }
 
-static void do_change_vnc(const char *target)
+static void do_change_vnc(const char *target, const char *arg)
 {
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
 	char password[9];
-	monitor_readline("Password: ", 1, password, sizeof(password));
+	if (arg) {
+	    strncpy(password, arg, sizeof(password));
+	    password[sizeof(password) - 1] = '\0';
+	} else
+	    monitor_readline("Password: ", 1, password, sizeof(password));
 	if (vnc_display_password(NULL, password) < 0)
 	    term_printf("could not set VNC server password\n");
     } else {

@@ -442,12 +446,12 @@ static void do_change_vnc(const char *target)
     }
 }
 
-static void do_change(const char *device, const char *target, const char *fmt)
+static void do_change(const char *device, const char *target, const char *arg)
 {
     if (strcmp(device, "vnc") == 0) {
-	do_change_vnc(target);
+	do_change_vnc(target, arg);
     } else {
-	do_change_block(device, target, fmt);
+	do_change_block(device, target, arg);
     }
 }

qemu-doc.texi +4 -4

index 1735d92..ca3b181 100644
@@ -1233,11 +1233,11 @@ and @var{options} are described at @ref{sec_invocation}. eg
 (qemu) change vnc localhost:1
 @end example
 
-@item change vnc password
+@item change vnc password [@var{password}]
 
-Change the password associated with the VNC server. The monitor will prompt for
-the new password to be entered. VNC passwords are only significant upto 8 letters.
-eg.
+Change the password associated with the VNC server. If the new password is not
+supplied, the monitor will prompt for it to be entered. VNC passwords are only
+significant up to 8 letters. eg
 
 @example
 (qemu) change vnc password

Chris Webb

author

2008-12-08 14:09 UTC #5 lore raw

PATCH RESEND v2 1/1 Accept password as an argument to 'change vnc password'

In reply to Chris Webb

Accept password as an argument to 'change vnc password' monitor command

This allows easier use of the change vnc password monitor command from management scripts, without having to implement expect(1)-like behaviour.

Signed-off-by Chris Webb <[email protected]>

---
 monitor.c     |   14 +++++++++-----
 qemu-doc.texi |    8 ++++----
 2 files changed, 13 insertions(+), 9 deletions(-)

monitor.c +9 -5

index a252838..f6a2783 100644
@@ -428,12 +428,16 @@ static void do_change_block(const char *device, const char *filename, const char
     qemu_key_check(bs, filename);
 }
 
-static void do_change_vnc(const char *target)
+static void do_change_vnc(const char *target, const char *arg)
 {
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
 	char password[9];
-	monitor_readline("Password: ", 1, password, sizeof(password));
+	if (arg) {
+	    strncpy(password, arg, sizeof(password));
+	    password[sizeof(password) - 1] = '\0';
+	} else
+	    monitor_readline("Password: ", 1, password, sizeof(password));
 	if (vnc_display_password(NULL, password) < 0)
 	    term_printf("could not set VNC server password\n");
     } else {

@@ -442,12 +446,12 @@ static void do_change_vnc(const char *target)
     }
 }
 
-static void do_change(const char *device, const char *target, const char *fmt)
+static void do_change(const char *device, const char *target, const char *arg)
 {
     if (strcmp(device, "vnc") == 0) {
-	do_change_vnc(target);
+	do_change_vnc(target, arg);
     } else {
-	do_change_block(device, target, fmt);
+	do_change_block(device, target, arg);
     }
 }

qemu-doc.texi +4 -4

index 1735d92..ca3b181 100644
@@ -1233,11 +1233,11 @@ and @var{options} are described at @ref{sec_invocation}. eg
 (qemu) change vnc localhost:1
 @end example
 
-@item change vnc password
+@item change vnc password [@var{password}]
 
-Change the password associated with the VNC server. The monitor will prompt for
-the new password to be entered. VNC passwords are only significant upto 8 letters.
-eg.
+Change the password associated with the VNC server. If the new password is not
+supplied, the monitor will prompt for it to be entered. VNC passwords are only
+significant up to 8 letters. eg
 
 @example
 (qemu) change vnc password

Anthony Liguori

Applied

2008-12-10 15:14 UTC #6 lore raw

Re: [RESEND] [PATCH v2] Accept password as an argument to 'change vnc password'

Quoted 7 lines · Chris Webb

Accept password as an argument to 'change vnc password' monitor command
This allows easier use of the change vnc password monitor command from management scripts, without having to implement expect(1)-like behaviour.
Signed-off-by Chris Webb <[email protected]>

Applied. Thanks.

Regards,

Anthony Liguori

Quoted 70 lines

---
 monitor.c     |   14 +++++++++-----
 qemu-doc.texi |    8 ++++----
 2 files changed, 13 insertions(+), 9 deletions(-)

monitor.c +9 -5

index a252838..f6a2783 100644
@@ -428,12 +428,16 @@ static void do_change_block(const char *device, const char *filename, const char
     qemu_key_check(bs, filename);
 }
 
-static void do_change_vnc(const char *target)
+static void do_change_vnc(const char *target, const char *arg)
 {
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
 	char password[9];
-	monitor_readline("Password: ", 1, password, sizeof(password));
+	if (arg) {
+	    strncpy(password, arg, sizeof(password));
+	    password[sizeof(password) - 1] = '\0';
+	} else
+	    monitor_readline("Password: ", 1, password, sizeof(password));
 	if (vnc_display_password(NULL, password) < 0)
 	    term_printf("could not set VNC server password\n");
     } else {

@@ -442,12 +446,12 @@ static void do_change_vnc(const char *target)
     }
 }
 
-static void do_change(const char *device, const char *target, const char *fmt)
+static void do_change(const char *device, const char *target, const char *arg)
 {
     if (strcmp(device, "vnc") == 0) {
-	do_change_vnc(target);
+	do_change_vnc(target, arg);
     } else {
-	do_change_block(device, target, fmt);
+	do_change_block(device, target, arg);
     }
 }

qemu-doc.texi +4 -5

index 1735d92..ca3b181 100644
@@ -1233,11 +1233,11 @@ and @var{options} are described at @ref{sec_invocation}. eg
 (qemu) change vnc localhost:1
 @end example
 
-@item change vnc password
+@item change vnc password [@var{password}]
 
-Change the password associated with the VNC server. The monitor will prompt for
-the new password to be entered. VNC passwords are only significant upto 8 letters.
-eg.
+Change the password associated with the VNC server. If the new password is not
+supplied, the monitor will prompt for it to be entered. VNC passwords are only
+significant up to 8 letters. eg
 
 @example
 (qemu) change vnc password
 
--

To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Chris Webb

author

2008-12-08 14:09 UTC #7 lore raw

PATCH RESEND v2 1/1 Fix off-by-one bug limiting VNC passwords to 7 chars

In reply to Chris Webb

Fix off-by-one bug limiting VNC passwords to 7 characters instead of 8

Signed-off-by Chris Webb <[email protected]>

---
 monitor.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

monitor.c +1 -2

index 22360fc..a252838 100644
@@ -433,8 +433,7 @@ static void do_change_vnc(const char *target)
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
 	char password[9];
-	monitor_readline("Password: ", 1, password, sizeof(password)-1);
-	password[sizeof(password)-1] = '\0';
+	monitor_readline("Password: ", 1, password, sizeof(password));
 	if (vnc_display_password(NULL, password) < 0)
 	    term_printf("could not set VNC server password\n");
     } else {

Anthony Liguori

Applied

2008-12-10 15:14 UTC #8 lore raw

Applied. Thanks.

Regards,

Anthony Liguori

Quoted 32 lines · Chris Webb

Fix off-by-one bug limiting VNC passwords to 7 characters instead of 8
monitor_readline expects buf_size to include the terminating \0, but do_change_vnc in monitor.c calls it as though it doesn't. The other site where monitor_readline reads a password (in vl.c) passes the buffer length correctly.
Signed-off-by Chris Webb <[email protected]>
---
 monitor.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
monitor.c +1 -3
index 22360fc..a252838 100644
@@ -433,8 +433,7 @@ static void do_change_vnc(const char *target)
     if (strcmp(target, "passwd") == 0 ||
 	strcmp(target, "password") == 0) {
 	char password[9];
-	monitor_readline("Password: ", 1, password, sizeof(password)-1);
-	password[sizeof(password)-1] = '\0';
+	monitor_readline("Password: ", 1, password, sizeof(password));
 	if (vnc_display_password(NULL, password) < 0)
 	    term_printf("could not set VNC server password\n");
     } else {
 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html