svn commit: r249096 - head/sys/rpc/rpcsec_gss

2013-04-04 Thread George V. Neville-Neil
Author: gnn
Date: Thu Apr  4 15:16:53 2013
New Revision: 249096
URL: http://svnweb.freebsd.org/changeset/base/249096

Log:
  Improve error handling when unwrapping received data.
  
  Submitted by: Rick Macklem
  MFC after:1 week

Modified:
  head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c

Modified: head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c
==
--- head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c   Thu Apr  4 15:03:12 2013
(r249095)
+++ head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c   Thu Apr  4 15:16:53 2013
(r249096)
@@ -208,6 +208,8 @@ m_trim(struct mbuf *m, int len)
struct mbuf *n;
int off;
 
+   if (m == NULL)
+   return;
n = m_getptr(m, len, off);
if (n) {
n-m_len = off;
@@ -251,10 +253,19 @@ xdr_rpc_gss_unwrap_data(struct mbuf **re
 * Extract the MIC and make it contiguous.
 */
cklen = get_uint32(results);
+   if (!results) {
+   m_freem(message);
+   return (FALSE);
+   }
KASSERT(cklen = MHLEN, (unexpected large GSS-API checksum));
mic = results;
-   if (cklen  mic-m_len)
+   if (cklen  mic-m_len) {
mic = m_pullup(mic, cklen);
+   if (!mic) {
+   m_freem(message);
+   return (FALSE);
+   }
+   }
if (cklen != RNDUP(cklen))
m_trim(mic, cklen);
 
@@ -272,6 +283,8 @@ xdr_rpc_gss_unwrap_data(struct mbuf **re
} else if (svc == rpc_gss_svc_privacy) {
/* Decode databody_priv. */
len = get_uint32(results);
+   if (!results)
+   return (FALSE);
 
/* Decrypt databody. */
message = results;
@@ -294,6 +307,8 @@ xdr_rpc_gss_unwrap_data(struct mbuf **re
 
/* Decode rpc_gss_data_t (sequence number + arguments). */
seq_num = get_uint32(message);
+   if (!message)
+   return (FALSE);

/* Verify sequence number. */
if (seq_num != seq) {
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r249096 - head/sys/rpc/rpcsec_gss

2013-04-04 Thread Gleb Smirnoff
  George, Rick,

On Thu, Apr 04, 2013 at 03:16:54PM +, George V. Neville-Neil wrote:
G Modified: head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c
G 
==
G --- head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.cThu Apr  4 15:03:12 
2013(r249095)
G +++ head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.cThu Apr  4 15:16:53 
2013(r249096)
G @@ -208,6 +208,8 @@ m_trim(struct mbuf *m, int len)
G  struct mbuf *n;
G  int off;
G  
G +if (m == NULL)
G +return;
G  n = m_getptr(m, len, off);
G  if (n) {
G  n-m_len = off;

I think the code will be much more rocksolid, if the function won't be
called with NULL argument.

-- 
Totus tuus, Glebius.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r249096 - head/sys/rpc/rpcsec_gss

2013-04-04 Thread Rick Macklem
Glebius wrote:
 George, Rick,
 
 On Thu, Apr 04, 2013 at 03:16:54PM +, George V. Neville-Neil
 wrote:
 G Modified: head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c
 G
 ==
 G --- head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c Thu Apr 4 15:03:12
 2013 (r249095)
 G +++ head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c Thu Apr 4 15:16:53
 2013 (r249096)
 G @@ -208,6 +208,8 @@ m_trim(struct mbuf *m, int len)
 G struct mbuf *n;
 G int off;
 G
 G + if (m == NULL)
 G + return;
Yep. If I recall correctly, my patch checked for a non-NULL argument
before calling m_trim(), but adding this sanity check seems like a
good idea to me, if only to avoid problems caused by modified calls
to m_trim() in the future.

I'd think you could just commit this without so@'s involvement? rick

 G n = m_getptr(m, len, off);
 G if (n) {
 G n-m_len = off;
 
 I think the code will be much more rocksolid, if the function won't be
 called with NULL argument.
 
 --
 Totus tuus, Glebius.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r249096 - head/sys/rpc/rpcsec_gss

2013-04-04 Thread Rick Macklem
Glebius wrote:
 George, Rick,
 
 On Thu, Apr 04, 2013 at 03:16:54PM +, George V. Neville-Neil
 wrote:
 G Modified: head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c
 G
 ==
 G --- head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c Thu Apr 4 15:03:12
 2013 (r249095)
 G +++ head/sys/rpc/rpcsec_gss/rpcsec_gss_prot.c Thu Apr 4 15:16:53
 2013 (r249096)
 G @@ -208,6 +208,8 @@ m_trim(struct mbuf *m, int len)
 G struct mbuf *n;
 G int off;
 G
 G + if (m == NULL)
 G + return;
 G n = m_getptr(m, len, off);
 G if (n) {
 G n-m_len = off;
 
 I think the code will be much more rocksolid, if the function won't be
 called with NULL argument.
 
Oops, my confusion. I thought you were suggesting the above change. I
suppose the callers should be fixed as well, but having the check here
seems like a good idea?

Feel free to add code to the callers to check for a NULL m arguement, rick

 --
 Totus tuus, Glebius.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org