[PATCH] Fix return check on gp_conv_gssx_to_name

[Robbie Harwood]

This bug was found by Coverity.
---
 proxy/src/gp_creds.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c
index 551b020..119c66f 100644
--- a/proxy/src/gp_creds.c
+++ b/proxy/src/gp_creds.c
@@ -578,7 +578,7 @@ uint32_t gp_add_krb5_creds(uint32_t *min,
         ret_min = gp_get_cred_environment(gpcall, desired_name, &req_name,
                                           &cred_usage, &cred_store);
     } else if (desired_name) {
-        ret_maj = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);
+        ret_min = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);
     }
     if (ret_min) {
         ret_maj = GSS_S_CRED_UNAVAIL;

[Simo Sorce]

On Mon, 2016-05-16 at 15:15 -0400, Robbie Harwood wrote:
> This bug was found by Coverity.
>
> proxy/src/gp_creds.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/proxy/src/gp_creds.c b/proxy/src/gp_creds.c
> index 551b020..119c66f 100644
> --- a/proxy/src/gp_creds.c
> +++ b/proxy/src/gp_creds.c
> @@ -578,7 +578,7 @@ uint32_t gp_add_krb5_creds(uint32_t *min,
>          ret_min = gp_get_cred_environment(gpcall, desired_name, &req_name,
>                                            &cred_usage, &cred_store);
>      } else if (desired_name) {
>    ret_maj = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);
>    ret_min = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);

nack, this clobbers the real ret_min.
we probably need to move the checks in he above if/else and have each
function check heir own.
We probably want to return the actual ret_maj instead of
GSS_S_CRED_UNAVAIL in the else branch anyway.

Simo.
>  }
>  if (ret_min) {
>      ret_maj = GSS_S_CRED_UNAVAIL;

[Robbie Harwood]

Simo Sorce  writes:
> On Mon, 2016-05-16 at 15:15 -0400, Robbie Harwood wrote:
> >    ret_maj = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);
> >    ret_min = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);
>
> nack, this clobbers the real ret_min.
> we probably need to move the checks in he above if/else and have each
> function check heir own.
> We probably want to return the actual ret_maj instead of
> GSS_S_CRED_UNAVAIL in the else branch anyway.

So it does.  How about this instead?

[Simo Sorce]

On Mon, 2016-05-16 at 18:40 -0400, Robbie Harwood wrote:
> Simo Sorce  writes:
> > On Mon, 2016-05-16 at 15:15 -0400, Robbie Harwood wrote:
> > >    ret_maj = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);
> > >    ret_min = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);
> >
> > nack, this clobbers the real ret_min.
> > we probably need to move the checks in he above if/else and have each
> > function check heir own.
> > We probably want to return the actual ret_maj instead of
> > GSS_S_CRED_UNAVAIL in the else branch anyway.
>
> So it does.  How about this instead?

Sounds better.

Simo.