netdev - Re: [PATCH 25/37] dccp: Feature activation handlers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080903043828.GC4105@gerrit.erg.abdn.ac.uk>
Date:	Wed, 3 Sep 2008 06:38:28 +0200
From:	Gerrit Renker <gerrit@....abdn.ac.uk>
To:	Wei Yongjun <yjwei@...fujitsu.com>
Cc:	dccp@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 25/37] dccp: Feature activation handlers

| > This patch provides the post-processing of feature negotiation state, after
| > the negotiation has completed.

<snip>
| >  
| > +int dccp_feat_activate_values(struct sock *sk, struct list_head *fn_list)
| > +{
| > +	struct dccp_sock *dp = dccp_sk(sk);
| > +	struct dccp_feat_entry *cur, *next;
| > +	int idx;
| > +	dccp_feat_val *fvals[DCCP_FEAT_SUPPORTED_MAX][2] = {
| > +		 [0 ... DCCP_FEAT_SUPPORTED_MAX-1] = { NULL, NULL }
| > +	};
| > +
| > +	list_for_each_entry(cur, fn_list, node) {
| > +
| > +		idx = dccp_feat_index(cur->feat_num);
| > +		if (idx < 0) {
| > +			DCCP_BUG("Unknown feature %u", cur->feat_num);
| > +			goto activation_failed;
| >   
| 
| idx < 0 is possible, if you goto activation_failed, the connection from
| endpoint which want to change feature we unkonwn, the connection will be
| always fail by reset. So I think it should just continue process the next
| feature(s).
| -----------------------------------
| if (idx < 0)
| continue;
| -----------------------------------
| 
| idx < 0 is happended when we recv a change option with unknown feature type.
| 
| 
No, since an unknown feature at this stage would most definitively be a bug.

The validity checking happens earlier:
 * for NN values every valid value is accepted as per RFC -
   this is ensured via dccp_feat_is_valid_nn_val();
 * for SP values at least the confirmed value must be valid -
   - this is checked in confirm_recv(),
   - setsockopt is protected against registering invalid feature values,
   - for CCIDs, additional a check for availability is made before
     entering negotiation.

These conditions (should) ensure that the above condition is never
triggered, the test is like an assert() clause.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html