[PATCH] powerpc/kvm: Handle the boundary condition correctly

From: "Aneesh Kumar K.V" <redacted>

We should be able to copy upto count bytes

Signed-off-by: Aneesh Kumar K.V <redacted>
---
 arch/powerpc/kvm/book3s_64_mmu_hv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 710d313..0ae6bb6 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c

@@ -1362,7 +1362,7 @@ static ssize_t kvm_htab_read(struct file *file, char __user *buf,
 	lbuf = (unsigned long __user *)buf;
 
 	nb = 0;
-	while (nb + sizeof(hdr) + HPTE_SIZE < count) {
+	while (nb + sizeof(hdr) + HPTE_SIZE <= count) {
 		/* Initialize header */
 		hptr = (struct kvm_get_htab_header __user *)buf;
 		hdr.n_valid = 0;

@@ -1385,7 +1385,7 @@ static ssize_t kvm_htab_read(struct file *file, char __user *buf,
 		/* Grab a series of valid entries */
 		while (i < kvm->arch.hpt_npte &&
 		       hdr.n_valid < 0xffff &&
-		       nb + HPTE_SIZE < count &&
+		       nb + HPTE_SIZE <= count &&
 		       record_hpte(flags, hptp, hpte, revp, 1, first_pass)) {
 			/* valid entry, write it out */
 			++hdr.n_valid;

-- 
1.8.1.2

On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:

From: "Aneesh Kumar K.V" <redacted>

Isn't this you?

=20
We should be able to copy upto count bytes

Why?


Alex

b/arch/powerpc/kvm/book3s_64_mmu_hv.c

char __user *buf,

char __user *buf,

		/* Grab a series of valid entries */
		while (i < kvm->arch.hpt_npte &&
		       hdr.n_valid < 0xffff &&
-		       nb + HPTE_SIZE < count &&
+		       nb + HPTE_SIZE <=3D count &&
		       record_hpte(flags, hptp, hpte, revp, 1, =

first_pass)) {

			/* valid entry, write it out */
			++hdr.n_valid;
--=20
1.8.1.2
=20

Alexander Graf [off-list ref] writes:

On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:

quoted

From: "Aneesh Kumar K.V" <redacted>

Isn't this you?

Yes. The patches are generated using git format-patch and sent by
git send-email. That's how it always created patches for me. I am not sure if
there is a config I can change to avoid having From:

quoted

We should be able to copy upto count bytes

Why?

Without this we end up doing

 +    struct kvm_get_htab_buf {
 +        struct kvm_get_htab_header header;
 +        /*
 +         * Older kernel required one extra byte.
 +         */
 +        unsigned long hpte[3];
 +    } hpte_buf;


even though we are only looking for one hpte entry.

http://mid.gmane.org/1376995766-16526-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com

Alex

quoted

Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 710d313..0ae6bb6 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c

@@ -1362,7 +1362,7 @@ static ssize_t kvm_htab_read(struct file *file, char __user *buf,

	lbuf = (unsigned long __user *)buf;

	nb = 0;
-	while (nb + sizeof(hdr) + HPTE_SIZE < count) {
+	while (nb + sizeof(hdr) + HPTE_SIZE <= count) {
		/* Initialize header */
		hptr = (struct kvm_get_htab_header __user *)buf;
		hdr.n_valid = 0;

@@ -1385,7 +1385,7 @@ static ssize_t kvm_htab_read(struct file *file, char __user *buf,

		/* Grab a series of valid entries */
		while (i < kvm->arch.hpt_npte &&
		       hdr.n_valid < 0xffff &&
-		       nb + HPTE_SIZE < count &&
+		       nb + HPTE_SIZE <= count &&
		       record_hpte(flags, hptp, hpte, revp, 1, first_pass)) {
			/* valid entry, write it out */
			++hdr.n_valid;
-- 
1.8.1.2

On Fri, 2013-08-23 at 09:01 +0530, Aneesh Kumar K.V wrote:

Alexander Graf [off-list ref] writes:

quoted

On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:

quoted

From: "Aneesh Kumar K.V" <redacted>

Isn't this you?

Yes. The patches are generated using git format-patch and sent by
git send-email. That's how it always created patches for me. I am not sure if
there is a config I can change to avoid having From:

Don't bother, that's perfectly fine, and git am will do the right thing.

Cheers,
Ben.

quoted

quoted

We should be able to copy upto count bytes

Why?

Without this we end up doing

 +    struct kvm_get_htab_buf {
 +        struct kvm_get_htab_header header;
 +        /*
 +         * Older kernel required one extra byte.
 +         */
 +        unsigned long hpte[3];
 +    } hpte_buf;


even though we are only looking for one hpte entry.

http://mid.gmane.org/1376995766-16526-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com

quoted

Alex

quoted

Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 710d313..0ae6bb6 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c

@@ -1362,7 +1362,7 @@ static ssize_t kvm_htab_read(struct file *file, char __user *buf,

	lbuf = (unsigned long __user *)buf;

	nb = 0;
-	while (nb + sizeof(hdr) + HPTE_SIZE < count) {

16 more quoted lines

On 23.08.2013, at 05:28, Benjamin Herrenschmidt wrote:

On Fri, 2013-08-23 at 09:01 +0530, Aneesh Kumar K.V wrote:

quoted

Alexander Graf [off-list ref] writes:
=20

quoted

On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:
=20

quoted

From: "Aneesh Kumar K.V" <redacted>

=20
Isn't this you?

=20
Yes. The patches are generated using git format-patch and sent by
git send-email. That's how it always created patches for me. I am not =

sure if

quoted

there is a config I can change to avoid having From:

=20
Don't bother, that's perfectly fine, and git am will do the right =

thing.

It will, but it's an indicator that something in his git config is =
misconfigured. Usually when git sees "Author =3D=3D Sender" it will omit =
the From: line.


Alex

On 23.08.2013, at 04:31, Aneesh Kumar K.V wrote:

Alexander Graf [off-list ref] writes:
=20

quoted

On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:
=20

quoted

From: "Aneesh Kumar K.V" <redacted>

=20
Isn't this you?

=20
Yes. The patches are generated using git format-patch and sent by
git send-email. That's how it always created patches for me. I am not =

sure if

there is a config I can change to avoid having From:
=20

quoted

=20

quoted

=20
We should be able to copy upto count bytes

=20
Why?
=20

=20
Without this we end up doing
=20
+    struct kvm_get_htab_buf {
+        struct kvm_get_htab_header header;
+        /*
+         * Older kernel required one extra byte.
+         */
+        unsigned long hpte[3];
+    } hpte_buf;
=20
=20
even though we are only looking for one hpte entry.

Ok, please give me an example with real numbers and why it breaks.


Alex

=20
=

http://mid.gmane.org/1376995766-16526-4-git-send-email-aneesh.kumar@linux.=
vnet.ibm.com

=20

quoted

=20
Alex
=20

quoted

=20
Signed-off-by: Aneesh Kumar K.V <redacted>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
=20

diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c =

b/arch/powerpc/kvm/book3s_64_mmu_hv.c

quoted

quoted

index 710d313..0ae6bb6 100644

--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c

@@ -1362,7 +1362,7 @@ static ssize_t kvm_htab_read(struct file =

*file, char __user *buf,

quoted

quoted

	lbuf =3D (unsigned long __user *)buf;
=20
	nb =3D 0;
-	while (nb + sizeof(hdr) + HPTE_SIZE < count) {
+	while (nb + sizeof(hdr) + HPTE_SIZE <=3D count) {
		/* Initialize header */
		hptr =3D (struct kvm_get_htab_header __user *)buf;
		hdr.n_valid =3D 0;

@@ -1385,7 +1385,7 @@ static ssize_t kvm_htab_read(struct file =

*file, char __user *buf,

quoted

quoted

		/* Grab a series of valid entries */
		while (i < kvm->arch.hpt_npte &&
		       hdr.n_valid < 0xffff &&
-		       nb + HPTE_SIZE < count &&
+		       nb + HPTE_SIZE <=3D count &&
		       record_hpte(flags, hptp, hpte, revp, 1, =

first_pass)) {

quoted

quoted

			/* valid entry, write it out */
			++hdr.n_valid;
--=20
1.8.1.2
=20

=20
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Alexander Graf [off-list ref] writes:

On 23.08.2013, at 04:31, Aneesh Kumar K.V wrote:

quoted

Alexander Graf [off-list ref] writes:

quoted

On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:

quoted

From: "Aneesh Kumar K.V" <redacted>

Isn't this you?

Yes. The patches are generated using git format-patch and sent by
git send-email. That's how it always created patches for me. I am not sure if
there is a config I can change to avoid having From:

quoted

quoted

We should be able to copy upto count bytes

Why?

Without this we end up doing

+    struct kvm_get_htab_buf {
+        struct kvm_get_htab_header header;
+        /*
+         * Older kernel required one extra byte.
+         */
+        unsigned long hpte[3];
+    } hpte_buf;


even though we are only looking for one hpte entry.

Ok, please give me an example with real numbers and why it breaks.

quoted

http://mid.gmane.org/1376995766-16526-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com

Didn't quiet get what you are looking for. As explained before, we now
need to pass an array with array size 3 even though we know we need to
read only 2 entries because kernel doesn't loop correctly.

-aneesh

On 26.08.2013, at 05:28, Aneesh Kumar K.V wrote:

Alexander Graf [off-list ref] writes:
=20

quoted

On 23.08.2013, at 04:31, Aneesh Kumar K.V wrote:
=20

quoted

Alexander Graf [off-list ref] writes:
=20

quoted

On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:
=20

quoted

From: "Aneesh Kumar K.V" <redacted>

=20
Isn't this you?

=20
Yes. The patches are generated using git format-patch and sent by
git send-email. That's how it always created patches for me. I am =

not sure if

quoted

quoted

there is a config I can change to avoid having From:
=20

quoted

=20

quoted

=20
We should be able to copy upto count bytes

=20
Why?
=20

=20
Without this we end up doing
=20
+    struct kvm_get_htab_buf {
+        struct kvm_get_htab_header header;
+        /*
+         * Older kernel required one extra byte.
+         */
+        unsigned long hpte[3];
+    } hpte_buf;
=20
=20
even though we are only looking for one hpte entry.

=20
Ok, please give me an example with real numbers and why it breaks.
=20

quoted

=20
=

http://mid.gmane.org/1376995766-16526-4-git-send-email-aneesh.kumar@linux.=
vnet.ibm.com

quoted

quoted

=20

=20
Didn't quiet get what you are looking for. As explained before, we now
need to pass an array with array size 3 even though we know we need to
read only 2 entries because kernel doesn't loop correctly.

But we need to do that regardless, because newer QEMU needs to be able =
to run on older kernels, no?


Alex

Alexander Graf [off-list ref] writes:

On 26.08.2013, at 05:28, Aneesh Kumar K.V wrote:

quoted

Alexander Graf [off-list ref] writes:

quoted

On 23.08.2013, at 04:31, Aneesh Kumar K.V wrote:

quoted

Alexander Graf [off-list ref] writes:

quoted

On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:

quoted

From: "Aneesh Kumar K.V" <redacted>

Isn't this you?

Yes. The patches are generated using git format-patch and sent by
git send-email. That's how it always created patches for me. I am not sure if
there is a config I can change to avoid having From:

quoted

quoted

We should be able to copy upto count bytes

Why?

Without this we end up doing

+    struct kvm_get_htab_buf {
+        struct kvm_get_htab_header header;
+        /*
+         * Older kernel required one extra byte.
+         */
+        unsigned long hpte[3];
+    } hpte_buf;


even though we are only looking for one hpte entry.

Ok, please give me an example with real numbers and why it breaks.

10 more quoted lines

yes. So use space will have to pass an array of size 3. But that should
not prevent us from fixing this right ?

-aneesh

Hi Alex,

"Aneesh Kumar K.V" [off-list ref] writes:

quoted

quoted

quoted

Ok, please give me an example with real numbers and why it breaks.

quoted

http://mid.gmane.org/1376995766-16526-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com

Didn't quiet get what you are looking for. As explained before, we now
need to pass an array with array size 3 even though we know we need to
read only 2 entries because kernel doesn't loop correctly.

But we need to do that regardless, because newer QEMU needs to be able to run on older kernels, no?

yes. So use space will have to pass an array of size 3. But that should
not prevent us from fixing this right ?

Do we still want this patch or should I drop this ?

-aneesh

On 25.09.2013, at 17:42, Aneesh Kumar K.V wrote:

=20
Hi Alex,
=20
"Aneesh Kumar K.V" [off-list ref] writes:

quoted

quoted

quoted

quoted

Ok, please give me an example with real numbers and why it breaks.
=20

quoted

=20
=

http://mid.gmane.org/1376995766-16526-4-git-send-email-aneesh.kumar@linux.=
vnet.ibm.com

quoted

quoted

quoted

quoted

quoted

=20

=20
Didn't quiet get what you are looking for. As explained before, we =

now

quoted

quoted

quoted

need to pass an array with array size 3 even though we know we need =

to

quoted

quoted

quoted

read only 2 entries because kernel doesn't loop correctly.

=20
But we need to do that regardless, because newer QEMU needs to be =

able to run on older kernels, no?

quoted

quoted

=20

=20
yes. So use space will have to pass an array of size 3. But that =

should

quoted

not prevent us from fixing this right ?
=20

=20
Do we still want this patch or should I drop this ?

If we really want it we need to be able to tell user space that we =
support this feature through a CAP, so that it can act accordingly. =
Given the additional complexity of this and the fact that we do want to =
support older KVM versions regardless, I don't think it's worth the =
hassle.


Alex