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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Gerrit Renker wrote:
| > 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.

This special case it is not as you said. The packet seq as following:

Endpoint A                    Endpoing B
REQUEST        --------->
(CHANGE L/unknow feature)
              <---------     RESPONSE
                             (CONFIRM R/empty unknow feature)
ACK            ---------->
                             call dccp_feat_activate_values()
                             -->print DCCP_BUG("Unknown feature %u", cur->feat_num);


After Endpoint B send REPONSE with empty CONFIRM R option, the unknow
feature is *still remain* in the feature list. dccp_feat_insert_opts() never clean up those features. So here we can get idx < 0. Features with is not needed is clean up
after active the feat value in dccp_feat_activate_values().

int dccp_feat_activate_values(struct sock *sk, struct list_head *fn_list)
{
   ...
   list_for_each_entry_safe(cur, next, fn_list, node)
       if (!cur->needs_confirm)
           dccp_feat_list_pop(cur);
   ...
}





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

[Index of Archives]     [Linux Kernel]     [IETF DCCP]     [Linux Networking]     [Git]     [Security]     [Linux Assembly]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux