RE: RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Lindenmaier, Goetz
Hi Vincent, 

I fixed the typo and added a remark to the first line of the bug text.

Best regards,
  Goetz.

> -Original Message-
> From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
> Sent: Mittwoch, 30. November 2016 17:10
> To: Lindenmaier, Goetz 
> Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
> Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding
> 
> Note that I have reviewed only the ECC/PKCS11 changes.
> You’ll need a JDK 9 reviewer from awt-dev for your remaining changes.
> 
> There is a minor typo in the commit message: s/ece/ECC
> 
> Also please change the Bug Summary in JBS to indicate that ECC and PKCS11
> changes are also present.
> 
> 
> > On 30 Nov 2016, at 15:41, Lindenmaier, Goetz 
> wrote:
> >
> > Hi Vincent,
> >
> > thanks for the quit review!
> > Good catch that I lost the change to p11_mutex.c ... I had to change
> > it and it fell out of my patches.
> 
> That looks fine.
> 
> 
> > I edited the Last Modified Date, and also updated the copyright messages.
> 
> Thanks.
> 
> 
> > New webrev:
> > http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/
> >
> > Best regards,
> >  Goetz.
> >
> > (Am I correct that your openJdk name is Vinnie?)
> 
> Yes.
> 
> 
> >
> >> -Original Message-
> >> From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
> >> Sent: Mittwoch, 30. November 2016 14:53
> >> To: Lindenmaier, Goetz 
> >> Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
> >> Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding
> >>
> >> Hello Goetz,
> >>
> >> Please modify the bug summary to reference ECC too.
> >> Your ECC changes look fine but the ‘Last Modified Date’ line in the 4 
> >> source
> >> code headers will need to be updated/added.
> >>
> >> BTW p11_mutex.c is listed below but appears to be missing from the
> webrev.
> >>
> >> Thanks.
> >>
> >>
> >>
> >>
> >>On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
> >> mailto:goetz.lindenma...@sap.com> >
> wrote:
> >>
> >>Hi,
> >>
> >>I’d like to propose a row of smaller fixes where code is noted down a
> >> bit questionable.
> >>SAP’s quality process requires that we fix these in our internal 
> >> delivery,
> >> and I
> >>Would like to share my fixes with openJdk.  Some of these fixes are of
> >> more
> >>theoretical nature as how I understand the code paths never allow the
> >>problematic situation, but fixing it nevertheless assures that nothing 
> >> is
> >>overseen if the code changes.  Most changes are in  libawt_xawt, some
> >>are in libsunec.
> >>
> >>I’d appreciate a review:
> >>http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/
> >>
> >>Changes in detail:
> >>
> >>awt_InputMethod.c:
> >>
> >>One might overrun the 100 byte fixed-size string statusWindow->status
> >> by copying text->string.multi_byte without checking the length.
> >>
> >>gtk3_interface.c:
> >>
> >>This less-than-zero comparison of an unsigned value is never true.
> >>
> >>Using uninitialized value color. Field color.alpha is uninitialized.
> >>E.g. used at gtk3_interface.c:2287.
> >>
> >>XToolkit.c
> >>
> >>Using uninitialized value ret_timeout.
> >>E.g. in XToolkit.c:6809.
> >>
> >>XWindow.c
> >>
> >>Argument is incompatible with corresponding format string conversion.
> >>
> >>splashscreen_sys.c
> >>
> >>Overflowed or truncated value (or a value computed from an
> >> overflowed or truncated value) (gdk_scale > 0) ? native_scale *
> >> (double)gdk_scale : native_scale used as return value.
> >>
> >>ec.c
> >>
> >>Using uninitialized value k.dp when calling mp_clear.
> >>
> >>ecdecode.c
> >>
> >>You might overrun the 291 byte fixed-size string genenc by copying
> >> curveParams->geny without checking the length.
> >>Added sanity check before doing the string concatenation.
> >>
> >>ecl_mult.c
> >>
> >>Using uninitialized value kt.flag when calling *group->point_mul. (The
> >> function pointer resolves to ec_GF2m_pt_mul_mont.)
> >>
> >>mpi.c
> >>
> >>Using uninitialized value s. Field s.flag is uninitialized when calling
> >> s_mp_exch.
> >>Using uninitialized value tmp. Field tmp.flag is uninitialized when
> >> calling s_mp_exch
> >>Using uninitialized value t.dp when calling mp_clear.
> >>
> >>p11_mutex.c
> >>
> >>Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
> >> uninitialized when calling memcpy.
> >>
> >>
> >>DataBufferNative.c
> >>
> >>Using uninitialized value lockInfo.rasBase when calling
> >> BN_GetPixelPointer.
> >>
> >>fontpath.c
> >>
> >>You might overrun the 512 byte fixed-size string fontDirPath by copying
> >> DirP->name[index] without checking the length.
> >>
> >



[9] RFR JDK-8157529 : Remove intermittent key from javax/net/ssl/DTLS/CipherSuite.java

2016-11-30 Thread Tim Du

Hi ,

Would you help to review the path for "8157529:Remove intermittent key 
from javax/net/ssl/DTLS/CipherSuite.java" , the intermittently failed 
issue was fixed by JDK-8167680 , '@key intermittent ' can be removed.Thanks.


JBS: https://bugs.openjdk.java.net/browse/JDK-8157529
webrev: http://cr.openjdk.java.net/~tidu/8157529/webrev.00/

Regards
Tim


Re: RFR: 8169335: Add a crypto.policy fallback in case Security Property 'crypto.policy' does not exist

2016-11-30 Thread Bradford Wetmore

I've updated to:

 * @run main/othervm CryptoPolicyFallback

I'll have a new review out shortly.

Brad


On 11/23/2016 2:29 AM, Wang Weijun wrote:

Hi Brad

I think I found a problem with the test. Before you set your local
java.security file, the system java.security file was already read (in
jtreg initialization) and limited was picked up. In fact, I modified the
java.security file of my own JDK to unlimited and the test fails.

This seems to show that we have to set the system property on the
command line. Either we provide a modified java.security with the test
like SeanM suggested, or we create it dynamically and manually launch a
new java process. I prefer the latter.

Thanks
Max

On 11/18/2016 1:46 AM, Bradford Wetmore wrote:

SeanM pointed out that we could do a:

@main -Djava.security.properties=xxx

but that would require storing a snapshot of java.security.  I think I
prefer it being dynamically generated.


Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Phil Race

Hi Goetz,


  DataBufferNative.c
Using uninitialized value lockInfo.rasBase when calling DBN_GetPixelPointer.


  75 lockInfo.resBase = NULL;

Did you actually compile this ? The variable is called "rasBase", not 
"resBase".


And strictly there is no problem since inside  DBN_GetPixelPointer
the code calls ops->Lock which should initialise this.
A "rasBase" of 0 isn't really any better than a random one ..

Also I don't see why there's a problem here and not in
the function immediately following since it is the exact same case.

-phil.

On 11/30/2016 10:00 AM, Sergey Bylokhov wrote:

cc 2d-dev.

On 30.11.16 18:41, Lindenmaier, Goetz wrote:

Hi Vincent,

thanks for the quit review!
Good catch that I lost the change to p11_mutex.c ... I had to change
it and it fell out of my patches.
I edited the Last Modified Date, and also updated the copyright 
messages.

New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/

Best regards,
  Goetz.

(Am I correct that your openJdk name is Vinnie?)


-Original Message-
From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 14:53
To: Lindenmaier, Goetz 
Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding

Hello Goetz,

Please modify the bug summary to reference ECC too.
Your ECC changes look fine but the ‘Last Modified Date’ line in the 
4 source

code headers will need to be updated/added.

BTW p11_mutex.c is listed below but appears to be missing from the 
webrev.


Thanks.




On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
mailto:goetz.lindenma...@sap.com> > wrote:

Hi,

I’d like to propose a row of smaller fixes where code is noted 
down a

bit questionable.
SAP’s quality process requires that we fix these in our internal 
delivery,

and I
Would like to share my fixes with openJdk.  Some of these fixes 
are of

more
theoretical nature as how I understand the code paths never 
allow the
problematic situation, but fixing it nevertheless assures that 
nothing is
overseen if the code changes.  Most changes are in libawt_xawt, 
some

are in libsunec.

I’d appreciate a review:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/

Changes in detail:

awt_InputMethod.c:

One might overrun the 100 byte fixed-size string 
statusWindow->status

by copying text->string.multi_byte without checking the length.

gtk3_interface.c:

This less-than-zero comparison of an unsigned value is never true.

Using uninitialized value color. Field color.alpha is 
uninitialized.

E.g. used at gtk3_interface.c:2287.

XToolkit.c

Using uninitialized value ret_timeout.
E.g. in XToolkit.c:6809.

XWindow.c

Argument is incompatible with corresponding format string 
conversion.


splashscreen_sys.c

Overflowed or truncated value (or a value computed from an
overflowed or truncated value) (gdk_scale > 0) ? native_scale *
(double)gdk_scale : native_scale used as return value.

ec.c

Using uninitialized value k.dp when calling mp_clear.

ecdecode.c

You might overrun the 291 byte fixed-size string genenc by copying
curveParams->geny without checking the length.
Added sanity check before doing the string concatenation.

ecl_mult.c

Using uninitialized value kt.flag when calling 
*group->point_mul. (The

function pointer resolves to ec_GF2m_pt_mul_mont.)

mpi.c

Using uninitialized value s. Field s.flag is uninitialized when 
calling

s_mp_exch.
Using uninitialized value tmp. Field tmp.flag is uninitialized when
calling s_mp_exch
Using uninitialized value t.dp when calling mp_clear.

p11_mutex.c

Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
uninitialized when calling memcpy.


DataBufferNative.c

Using uninitialized value lockInfo.rasBase when calling
BN_GetPixelPointer.

fontpath.c

You might overrun the 512 byte fixed-size string fontDirPath by 
copying

DirP->name[index] without checking the length.










RE: RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Langer, Christoph
Hi Goetz,

I have some small remarks.

src/java.desktop/unix/native/common/awt/fontpath.c:

247 fontDirPath[sizeof(fontDirPath)-1] = '\0';
-> you should add spaces left and right of '-'
248 strncat(fontDirPath, "/fonts.dir", sizeof(fontDirPath) - 
strlen(fontDirPath));
-> I think you need to subtract 1 from the 3rd parameter of strncat ('size_t 
n') because man page says:
"If src contains n or more bytes, strncat() writes n+1 bytes to dest (n from 
src plus the terminating null byte). Therefore, the size of dest must be at 
least strlen(dest)+n+1."

src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c:

add spaces around '-' in the array index

The rest looks fine to me.

Best regards
Christoph

> -Original Message-
> From: security-dev [mailto:security-dev-boun...@openjdk.java.net] On Behalf
> Of Lindenmaier, Goetz
> Sent: Mittwoch, 30. November 2016 16:41
> To: Vincent Ryan 
> Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
> Subject: RE: RFR(M): 8170525: Fix minor issues in awt coding
> 
> Hi Vincent,
> 
> thanks for the quit review!
> Good catch that I lost the change to p11_mutex.c ... I had to change
> it and it fell out of my patches.
> I edited the Last Modified Date, and also updated the copyright messages.
> New webrev:
> http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/
> 
> Best regards,
>   Goetz.
> 
> (Am I correct that your openJdk name is Vinnie?)
> 
> > -Original Message-
> > From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
> > Sent: Mittwoch, 30. November 2016 14:53
> > To: Lindenmaier, Goetz 
> > Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
> > Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding
> >
> > Hello Goetz,
> >
> > Please modify the bug summary to reference ECC too.
> > Your ECC changes look fine but the ‘Last Modified Date’ line in the 4 source
> > code headers will need to be updated/added.
> >
> > BTW p11_mutex.c is listed below but appears to be missing from the webrev.
> >
> > Thanks.
> >
> >
> >
> >
> > On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
> > mailto:goetz.lindenma...@sap.com> > wrote:
> >
> > Hi,
> >
> > I’d like to propose a row of smaller fixes where code is noted down a
> > bit questionable.
> > SAP’s quality process requires that we fix these in our internal 
> > delivery,
> > and I
> > Would like to share my fixes with openJdk.  Some of these fixes are of
> > more
> > theoretical nature as how I understand the code paths never allow the
> > problematic situation, but fixing it nevertheless assures that nothing 
> > is
> > overseen if the code changes.  Most changes are in  libawt_xawt, some
> > are in libsunec.
> >
> > I’d appreciate a review:
> > http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/
> >
> > Changes in detail:
> >
> > awt_InputMethod.c:
> >
> > One might overrun the 100 byte fixed-size string statusWindow->status
> > by copying text->string.multi_byte without checking the length.
> >
> > gtk3_interface.c:
> >
> > This less-than-zero comparison of an unsigned value is never true.
> >
> > Using uninitialized value color. Field color.alpha is uninitialized.
> > E.g. used at gtk3_interface.c:2287.
> >
> > XToolkit.c
> >
> > Using uninitialized value ret_timeout.
> > E.g. in XToolkit.c:6809.
> >
> > XWindow.c
> >
> > Argument is incompatible with corresponding format string conversion.
> >
> > splashscreen_sys.c
> >
> > Overflowed or truncated value (or a value computed from an
> > overflowed or truncated value) (gdk_scale > 0) ? native_scale *
> > (double)gdk_scale : native_scale used as return value.
> >
> > ec.c
> >
> > Using uninitialized value k.dp when calling mp_clear.
> >
> > ecdecode.c
> >
> > You might overrun the 291 byte fixed-size string genenc by copying
> > curveParams->geny without checking the length.
> > Added sanity check before doing the string concatenation.
> >
> > ecl_mult.c
> >
> > Using uninitialized value kt.flag when calling *group->point_mul. (The
> > function pointer resolves to ec_GF2m_pt_mul_mont.)
> >
> > mpi.c
> >
> > Using uninitialized value s. Field s.flag is uninitialized when calling
> > s_mp_exch.
> > Using uninitialized value tmp. Field tmp.flag is uninitialized when
> > calling s_mp_exch
> > Using uninitialized value t.dp when calling mp_clear.
> >
> > p11_mutex.c
> >
> > Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
> > uninitialized when calling memcpy.
> >
> >
> > DataBufferNative.c
> >
> > Using uninitialized value lockInfo.rasBase when calling
> > BN_GetPixelPointer.
> >
> > fontpath.c
> >
> > You might overrun the 512 byte fixed-size string fontDirPath by copying
> > DirP->name[index] without checking the length.
> >



Re: RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Sergey Bylokhov

cc 2d-dev.

On 30.11.16 18:41, Lindenmaier, Goetz wrote:

Hi Vincent,

thanks for the quit review!
Good catch that I lost the change to p11_mutex.c ... I had to change
it and it fell out of my patches.
I edited the Last Modified Date, and also updated the copyright messages.
New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/

Best regards,
  Goetz.

(Am I correct that your openJdk name is Vinnie?)


-Original Message-
From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 14:53
To: Lindenmaier, Goetz 
Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding

Hello Goetz,

Please modify the bug summary to reference ECC too.
Your ECC changes look fine but the ‘Last Modified Date’ line in the 4 source
code headers will need to be updated/added.

BTW p11_mutex.c is listed below but appears to be missing from the webrev.

Thanks.




On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
mailto:goetz.lindenma...@sap.com> > wrote:

Hi,

I’d like to propose a row of smaller fixes where code is noted down a
bit questionable.
SAP’s quality process requires that we fix these in our internal 
delivery,
and I
Would like to share my fixes with openJdk.  Some of these fixes are of
more
theoretical nature as how I understand the code paths never allow the
problematic situation, but fixing it nevertheless assures that nothing 
is
overseen if the code changes.  Most changes are in  libawt_xawt, some
are in libsunec.

I’d appreciate a review:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/

Changes in detail:

awt_InputMethod.c:

One might overrun the 100 byte fixed-size string statusWindow->status
by copying text->string.multi_byte without checking the length.

gtk3_interface.c:

This less-than-zero comparison of an unsigned value is never true.

Using uninitialized value color. Field color.alpha is uninitialized.
E.g. used at gtk3_interface.c:2287.

XToolkit.c

Using uninitialized value ret_timeout.
E.g. in XToolkit.c:6809.

XWindow.c

Argument is incompatible with corresponding format string conversion.

splashscreen_sys.c

Overflowed or truncated value (or a value computed from an
overflowed or truncated value) (gdk_scale > 0) ? native_scale *
(double)gdk_scale : native_scale used as return value.

ec.c

Using uninitialized value k.dp when calling mp_clear.

ecdecode.c

You might overrun the 291 byte fixed-size string genenc by copying
curveParams->geny without checking the length.
Added sanity check before doing the string concatenation.

ecl_mult.c

Using uninitialized value kt.flag when calling *group->point_mul. (The
function pointer resolves to ec_GF2m_pt_mul_mont.)

mpi.c

Using uninitialized value s. Field s.flag is uninitialized when calling
s_mp_exch.
Using uninitialized value tmp. Field tmp.flag is uninitialized when
calling s_mp_exch
Using uninitialized value t.dp when calling mp_clear.

p11_mutex.c

Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
uninitialized when calling memcpy.


DataBufferNative.c

Using uninitialized value lockInfo.rasBase when calling
BN_GetPixelPointer.

fontpath.c

You might overrun the 512 byte fixed-size string fontDirPath by copying
DirP->name[index] without checking the length.






--
Best regards, Sergey.


Re: RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Vincent Ryan
Note that I have reviewed only the ECC/PKCS11 changes.
You’ll need a JDK 9 reviewer from awt-dev for your remaining changes.

There is a minor typo in the commit message: s/ece/ECC

Also please change the Bug Summary in JBS to indicate that ECC and PKCS11 
changes are also present.


> On 30 Nov 2016, at 15:41, Lindenmaier, Goetz  
> wrote:
> 
> Hi Vincent, 
> 
> thanks for the quit review!  
> Good catch that I lost the change to p11_mutex.c ... I had to change
> it and it fell out of my patches.

That looks fine.


> I edited the Last Modified Date, and also updated the copyright messages.

Thanks.


> New webrev:
> http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/
> 
> Best regards,
>  Goetz.
> 
> (Am I correct that your openJdk name is Vinnie?)

Yes.


> 
>> -Original Message-
>> From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
>> Sent: Mittwoch, 30. November 2016 14:53
>> To: Lindenmaier, Goetz 
>> Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
>> Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding
>> 
>> Hello Goetz,
>> 
>> Please modify the bug summary to reference ECC too.
>> Your ECC changes look fine but the ‘Last Modified Date’ line in the 4 source
>> code headers will need to be updated/added.
>> 
>> BTW p11_mutex.c is listed below but appears to be missing from the webrev.
>> 
>> Thanks.
>> 
>> 
>> 
>> 
>>  On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
>> mailto:goetz.lindenma...@sap.com> > wrote:
>> 
>>  Hi,
>> 
>>  I’d like to propose a row of smaller fixes where code is noted down a
>> bit questionable.
>>  SAP’s quality process requires that we fix these in our internal 
>> delivery,
>> and I
>>  Would like to share my fixes with openJdk.  Some of these fixes are of
>> more
>>  theoretical nature as how I understand the code paths never allow the
>>  problematic situation, but fixing it nevertheless assures that nothing 
>> is
>>  overseen if the code changes.  Most changes are in  libawt_xawt, some
>>  are in libsunec.
>> 
>>  I’d appreciate a review:
>>  http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/
>> 
>>  Changes in detail:
>> 
>>  awt_InputMethod.c:
>> 
>>  One might overrun the 100 byte fixed-size string statusWindow->status
>> by copying text->string.multi_byte without checking the length.
>> 
>>  gtk3_interface.c:
>> 
>>  This less-than-zero comparison of an unsigned value is never true.
>> 
>>  Using uninitialized value color. Field color.alpha is uninitialized.
>>  E.g. used at gtk3_interface.c:2287.
>> 
>>  XToolkit.c
>> 
>>  Using uninitialized value ret_timeout.
>>  E.g. in XToolkit.c:6809.
>> 
>>  XWindow.c
>> 
>>  Argument is incompatible with corresponding format string conversion.
>> 
>>  splashscreen_sys.c
>> 
>>  Overflowed or truncated value (or a value computed from an
>> overflowed or truncated value) (gdk_scale > 0) ? native_scale *
>> (double)gdk_scale : native_scale used as return value.
>> 
>>  ec.c
>> 
>>  Using uninitialized value k.dp when calling mp_clear.
>> 
>>  ecdecode.c
>> 
>>  You might overrun the 291 byte fixed-size string genenc by copying
>> curveParams->geny without checking the length.
>>  Added sanity check before doing the string concatenation.
>> 
>>  ecl_mult.c
>> 
>>  Using uninitialized value kt.flag when calling *group->point_mul. (The
>> function pointer resolves to ec_GF2m_pt_mul_mont.)
>> 
>>  mpi.c
>> 
>>  Using uninitialized value s. Field s.flag is uninitialized when calling
>> s_mp_exch.
>>  Using uninitialized value tmp. Field tmp.flag is uninitialized when
>> calling s_mp_exch
>>  Using uninitialized value t.dp when calling mp_clear.
>> 
>>  p11_mutex.c
>> 
>>  Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
>> uninitialized when calling memcpy.
>> 
>> 
>>  DataBufferNative.c
>> 
>>  Using uninitialized value lockInfo.rasBase when calling
>> BN_GetPixelPointer.
>> 
>>  fontpath.c
>> 
>>  You might overrun the 512 byte fixed-size string fontDirPath by copying
>> DirP->name[index] without checking the length.
>> 
> 



RE: RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Lindenmaier, Goetz
Hi Vincent, 

thanks for the quit review!  
Good catch that I lost the change to p11_mutex.c ... I had to change
it and it fell out of my patches.
I edited the Last Modified Date, and also updated the copyright messages.
New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/

Best regards,
  Goetz.

(Am I correct that your openJdk name is Vinnie?)

> -Original Message-
> From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
> Sent: Mittwoch, 30. November 2016 14:53
> To: Lindenmaier, Goetz 
> Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
> Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding
> 
> Hello Goetz,
> 
> Please modify the bug summary to reference ECC too.
> Your ECC changes look fine but the ‘Last Modified Date’ line in the 4 source
> code headers will need to be updated/added.
> 
> BTW p11_mutex.c is listed below but appears to be missing from the webrev.
> 
> Thanks.
> 
> 
> 
> 
>   On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
> mailto:goetz.lindenma...@sap.com> > wrote:
> 
>   Hi,
> 
>   I’d like to propose a row of smaller fixes where code is noted down a
> bit questionable.
>   SAP’s quality process requires that we fix these in our internal 
> delivery,
> and I
>   Would like to share my fixes with openJdk.  Some of these fixes are of
> more
>   theoretical nature as how I understand the code paths never allow the
>   problematic situation, but fixing it nevertheless assures that nothing 
> is
>   overseen if the code changes.  Most changes are in  libawt_xawt, some
>   are in libsunec.
> 
>   I’d appreciate a review:
>   http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/
> 
>   Changes in detail:
> 
>   awt_InputMethod.c:
> 
>   One might overrun the 100 byte fixed-size string statusWindow->status
> by copying text->string.multi_byte without checking the length.
> 
>   gtk3_interface.c:
> 
>   This less-than-zero comparison of an unsigned value is never true.
> 
>   Using uninitialized value color. Field color.alpha is uninitialized.
>   E.g. used at gtk3_interface.c:2287.
> 
>   XToolkit.c
> 
>   Using uninitialized value ret_timeout.
>   E.g. in XToolkit.c:6809.
> 
>   XWindow.c
> 
>   Argument is incompatible with corresponding format string conversion.
> 
>   splashscreen_sys.c
> 
>   Overflowed or truncated value (or a value computed from an
> overflowed or truncated value) (gdk_scale > 0) ? native_scale *
> (double)gdk_scale : native_scale used as return value.
> 
>   ec.c
> 
>   Using uninitialized value k.dp when calling mp_clear.
> 
>   ecdecode.c
> 
>   You might overrun the 291 byte fixed-size string genenc by copying
> curveParams->geny without checking the length.
>   Added sanity check before doing the string concatenation.
> 
>   ecl_mult.c
> 
>   Using uninitialized value kt.flag when calling *group->point_mul. (The
> function pointer resolves to ec_GF2m_pt_mul_mont.)
> 
>   mpi.c
> 
>   Using uninitialized value s. Field s.flag is uninitialized when calling
> s_mp_exch.
>   Using uninitialized value tmp. Field tmp.flag is uninitialized when
> calling s_mp_exch
>   Using uninitialized value t.dp when calling mp_clear.
> 
>   p11_mutex.c
> 
>   Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
> uninitialized when calling memcpy.
> 
> 
>   DataBufferNative.c
> 
>   Using uninitialized value lockInfo.rasBase when calling
> BN_GetPixelPointer.
> 
>   fontpath.c
> 
>   You might overrun the 512 byte fixed-size string fontDirPath by copying
> DirP->name[index] without checking the length.
> 



Re: RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Vincent Ryan
Hello Goetz,

Please modify the bug summary to reference ECC too.
Your ECC changes look fine but the ‘Last Modified Date’ line in the 4 source 
code headers will need to be updated/added.

BTW p11_mutex.c is listed below but appears to be missing from the webrev.

Thanks.



> On 30 Nov 2016, at 13:12, Lindenmaier, Goetz  
> wrote:
> 
> Hi, 
>  
> I’d like to propose a row of smaller fixes where code is noted down a bit 
> questionable.
> SAP’s quality process requires that we fix these in our internal delivery, 
> and I
> Would like to share my fixes with openJdk.  Some of these fixes are of more
> theoretical nature as how I understand the code paths never allow the
> problematic situation, but fixing it nevertheless assures that nothing is
> overseen if the code changes.  Most changes are in  libawt_xawt, some
> are in libsunec.
>  
> I’d appreciate a review:
> http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/ 
> 
>  
> Changes in detail:
> 
> awt_InputMethod.c: 
> 
> One might overrun the 100 byte fixed-size string statusWindow->status by 
> copying text->string.multi_byte without checking the length. 
> 
> gtk3_interface.c: 
> 
> This less-than-zero comparison of an unsigned value is never true. 
> 
> Using uninitialized value color. Field color.alpha is uninitialized. 
> E.g. used at gtk3_interface.c:2287. 
> 
> XToolkit.c 
> 
> Using uninitialized value ret_timeout. 
> E.g. in XToolkit.c:6809. 
> 
> XWindow.c 
> 
> Argument is incompatible with corresponding format string conversion. 
> 
> splashscreen_sys.c 
> 
> Overflowed or truncated value (or a value computed from an overflowed or 
> truncated value) (gdk_scale > 0) ? native_scale * (double)gdk_scale : 
> native_scale used as return value. 
> 
> ec.c 
> 
> Using uninitialized value k.dp when calling mp_clear. 
> 
> ecdecode.c 
> 
> You might overrun the 291 byte fixed-size string genenc by copying 
> curveParams->geny without checking the length. 
> Added sanity check before doing the string concatenation. 
> 
> ecl_mult.c 
> 
> Using uninitialized value kt.flag when calling *group->point_mul. (The 
> function pointer resolves to ec_GF2m_pt_mul_mont.) 
> 
> mpi.c 
> 
> Using uninitialized value s. Field s.flag is uninitialized when calling 
> s_mp_exch. 
> Using uninitialized value tmp. Field tmp.flag is uninitialized when calling 
> s_mp_exch 
> Using uninitialized value t.dp when calling mp_clear. 
> 
> p11_mutex.c 
> 
> Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is 
> uninitialized when calling memcpy. 
> 
> 
> DataBufferNative.c 
> 
> Using uninitialized value lockInfo.rasBase when calling BN_GetPixelPointer. 
> 
> fontpath.c 
> 
> You might overrun the 512 byte fixed-size string fontDirPath by copying 
> DirP->name[index] without checking the length.



RFR(M): 8170525: Fix minor issues in awt coding

2016-11-30 Thread Lindenmaier, Goetz
Hi,

I'd like to propose a row of smaller fixes where code is noted down a bit 
questionable.
SAP's quality process requires that we fix these in our internal delivery, and I
Would like to share my fixes with openJdk.  Some of these fixes are of more
theoretical nature as how I understand the code paths never allow the
problematic situation, but fixing it nevertheless assures that nothing is
overseen if the code changes.  Most changes are in  libawt_xawt, some
are in libsunec.

I'd appreciate a review:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/

Changes in detail:

awt_InputMethod.c:

One might overrun the 100 byte fixed-size string statusWindow->status by 
copying text->string.multi_byte without checking the length.

gtk3_interface.c:

This less-than-zero comparison of an unsigned value is never true.

Using uninitialized value color. Field color.alpha is uninitialized.
E.g. used at gtk3_interface.c:2287.

XToolkit.c

Using uninitialized value ret_timeout.
E.g. in XToolkit.c:6809.

XWindow.c

Argument is incompatible with corresponding format string conversion.

splashscreen_sys.c

Overflowed or truncated value (or a value computed from an overflowed or 
truncated value) (gdk_scale > 0) ? native_scale * (double)gdk_scale : 
native_scale used as return value.

ec.c

Using uninitialized value k.dp when calling mp_clear.

ecdecode.c

You might overrun the 291 byte fixed-size string genenc by copying 
curveParams->geny without checking the length.
Added sanity check before doing the string concatenation.

ecl_mult.c

Using uninitialized value kt.flag when calling *group->point_mul. (The function 
pointer resolves to ec_GF2m_pt_mul_mont.)

mpi.c

Using uninitialized value s. Field s.flag is uninitialized when calling 
s_mp_exch.
Using uninitialized value tmp. Field tmp.flag is uninitialized when calling 
s_mp_exch
Using uninitialized value t.dp when calling mp_clear.

p11_mutex.c

Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is 
uninitialized when calling memcpy.


DataBufferNative.c

Using uninitialized value lockInfo.rasBase when calling BN_GetPixelPointer.

fontpath.c

You might overrun the 512 byte fixed-size string fontDirPath by copying 
DirP->name[index] without checking the length.


Re: [9] RFR 8170247: java/security/SecureRandom/ApiTest fails when run with unlimited policy.

2016-11-30 Thread Wang Weijun

Change looks fine.

One nit: the extra space at the beginning of line 24 looks strange.

Thanks
Max

On 11/30/2016 5:27 PM, Sibabrata Sahoo wrote:

Hi,



Please review the patch for,



JBS: https://bugs.openjdk.java.net/browse/JDK-8170247

Webrev: http://cr.openjdk.java.net/~ssahoo/8170247/webrev.00/



Description:

The Test was failing to handle the expected failure for invalid
parameters, when the SecureRandom parameter size is > algorithm running
with unlimited policy. I have modified the condition to handle the
expected failures correctly. The change is only applicable to the “if”
condition(Line:200). The other 2 lines are minor changes related to
statement performance and renaming a variable.



Thanks,

Siba





[9] RFR 8170247: java/security/SecureRandom/ApiTest fails when run with unlimited policy.

2016-11-30 Thread Sibabrata Sahoo
Hi,

 

Please review the patch for,

 

JBS: https://bugs.openjdk.java.net/browse/JDK-8170247

Webrev: http://cr.openjdk.java.net/~ssahoo/8170247/webrev.00/

 

Description:

The Test was failing to handle the expected failure for invalid parameters, 
when the SecureRandom parameter size is > algorithm running with unlimited 
policy. I have modified the condition to handle the expected failures 
correctly. The change is only applicable to the "if" condition(Line:200). The 
other 2 lines are minor changes related to statement performance and renaming a 
variable.

 

Thanks,

Siba