Thread: Possible Null dereference in code.

  1. #1
    Registered User
    Join Date
    Oct 2019
    Posts
    77

    Possible Null dereference in code.

    I'm looking at some Linux code and something seems strange to me.

    In the code below, it is not clear to me what the else clause achieves and whether it is broken...

    Code:
    } else {
    			/* Nothing to do. Next chunk in the packet, please. */
    			ch = (struct sctp_chunkhdr *)chunk->chunk_end;
    			/* Force chunk->skb->data to chunk->chunk_end.  */
    			skb_pull(chunk->skb, chunk->chunk_end - chunk->skb->data);
    			/* We are guaranteed to pull a SCTP header. */
    		}
    Isn't this the code above trying to deference a null pointer in the code below

    Code:
    struct sctp_chunk *sctp_inq_pop(struct sctp_inq *queue)
    {
    	struct sctp_chunk *chunk;
    	struct sctp_chunkhdr *ch = NULL;
    
    
    	/* The assumption is that we are safe to process the chunks
    	 * at this time.
    	 */
    
    
    	chunk = queue->in_progress;
    	if (chunk) {
    		/* There is a packet that we have been working on.
    		 * Any post processing work to do before we move on?
    		 */
    		if (chunk->singleton ||
    		    chunk->end_of_packet ||
    		    chunk->pdiscard) {
    			if (chunk->head_skb == chunk->skb) {
    				chunk->skb = skb_shinfo(chunk->skb)->frag_list;
    				goto new_skb;
    			}
    			if (chunk->skb->next) {
    				chunk->skb = chunk->skb->next;
    				goto new_skb;
    			}
    
    
    			if (chunk->head_skb)
    				chunk->skb = chunk->head_skb;
    			sctp_chunk_free(chunk);
    			chunk = queue->in_progress = NULL;
    		} else {
    			/* Nothing to do. Next chunk in the packet, please. */
    			ch = (struct sctp_chunkhdr *)chunk->chunk_end;
    			/* Force chunk->skb->data to chunk->chunk_end.  */
    			skb_pull(chunk->skb, chunk->chunk_end - chunk->skb->data);
    			/* We are guaranteed to pull a SCTP header. */
    		}
    	}
    
    
    	/* Do we need to take the next packet out of the queue to process? */
    	if (!chunk) {
    		struct list_head *entry;
    
    
    next_chunk:
    		/* Is the queue empty?  */
    		entry = sctp_list_dequeue(&queue->in_chunk_list);
    		if (!entry)
    			return NULL;
    
    
    		chunk = list_entry(entry, struct sctp_chunk, list);
    
    
    		if (skb_is_gso(chunk->skb) && skb_is_gso_sctp(chunk->skb)) {
    			/* GSO-marked skbs but without frags, handle
    			 * them normally
    			 */
    			if (skb_shinfo(chunk->skb)->frag_list)
    				chunk->head_skb = chunk->skb;
    
    
    			/* skbs with "cover letter" */
    			if (chunk->head_skb && chunk->skb->data_len == chunk->skb->len)
    				chunk->skb = skb_shinfo(chunk->skb)->frag_list;
    
    
    			if (WARN_ON(!chunk->skb)) {
    				__SCTP_INC_STATS(dev_net(chunk->skb->dev), SCTP_MIB_IN_PKT_DISCARDS);
    				sctp_chunk_free(chunk);
    				goto next_chunk;
    			}
    		}
    
    
    		if (chunk->asoc)
    			sock_rps_save_rxhash(chunk->asoc->base.sk, chunk->skb);
    
    
    		queue->in_progress = chunk;
    
    
    new_skb:
    		/* This is the first chunk in the packet.  */
    		ch = (struct sctp_chunkhdr *)chunk->skb->data;
    		chunk->singleton = 1;
    		chunk->data_accepted = 0;
    		chunk->pdiscard = 0;
    		chunk->auth = 0;
    		chunk->has_asconf = 0;
    		chunk->end_of_packet = 0;
    		chunk->length = 0;
    		if (chunk->head_skb) {
    			struct sctp_input_cb
    				*cb = SCTP_INPUT_CB(chunk->skb),
    				*head_cb = SCTP_INPUT_CB(chunk->head_skb);
    
    
    			cb->chunk = head_cb->chunk;
    			cb->af = head_cb->af;
    		}
    	}
    
    
    	chunk->chunk_hdr = ch;
    	chunk->chunk_end = ((__u8 *)ch) + SCTP_PAD4(ntohs(ch->length));
    	skb_pull(chunk->skb, sizeof(*ch));
    	chunk->subh.v = NULL; /* Subheader is no longer valid.  */
    
    
    	if (chunk->chunk_end + sizeof(*ch) <= skb_tail_pointer(chunk->skb)) {
    		/* This is not a singleton */
    		chunk->singleton = 0;
    	} else if (chunk->chunk_end > skb_tail_pointer(chunk->skb)) {
    		/* Discard inside state machine. */
    		chunk->pdiscard = 1;
    		chunk->chunk_end = skb_tail_pointer(chunk->skb);
    	} else {
    		/* We are at the end of the packet, so mark the chunk
    		 * in case we need to send a SACK.
    		 */
    		chunk->end_of_packet = 1;
    	}
    
    
    	pr_debug("+++sctp_inq_pop+++ chunk:%p[%s], length:%d, skb->len:%d\n",
    		 chunk, sctp_cname(SCTP_ST_CHUNK(chunk->chunk_hdr->type)),
    		 ntohs(chunk->chunk_hdr->length), chunk->skb->len);
    
    
    	return chunk;
    }

    I am not sure, myself, because no bugs have been reported yet related to this but it is possible that this code path never gets taken.

    Any input is very much appreciated.

    Thanks in advance!

  2. #2
    Registered User
    Join Date
    Feb 2022
    Location
    Canada, PEI
    Posts
    64
    Could you use valgrind to verify this?

    Valgrind Home

  3. #3
    Registered User
    Join Date
    Dec 2017
    Posts
    1,422
    Why do you think it's dereferencing (not deference!) a null pointer?
    The else on line 34 pairs with the if on line 17, not the if on line 13.
    A long time ago being crazy meant something. - Charles Manson

  4. #4
    Registered User
    Join Date
    Oct 2019
    Posts
    77
    Quote Originally Posted by john.c View Post
    Why do you think it's dereferencing (not deference!) a null pointer?
    The else on line 34 pairs with the if on line 17, not the if on line 13.
    Sorry about the typo.

    You are right. Thanks

    Quote Originally Posted by G4143 View Post
    Could you use valgrind to verify this?

    Valgrind Home
    I don't think it possible to debug the kernel with valgrind but there's kmemleak(which reports memory leaks). And, even if it was, it probably would have been painfully difficult to setup(as weill all other kernel debugging setups).

    Thanks for you time, all the same!
    Last edited by ghoul; 05-07-2022 at 06:53 AM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Null bytes in code
    By Aslaville in forum Tech Board
    Replies: 2
    Last Post: 05-11-2015, 12:12 PM
  2. XNA code error Value cannot be null.
    By Demipimp in forum C# Programming
    Replies: 6
    Last Post: 05-03-2012, 05:32 PM
  3. null pointer dereference
    By qwertylurker in forum C Programming
    Replies: 3
    Last Post: 03-14-2011, 12:06 AM
  4. Dereference a member of NULL ?
    By kbrandt in forum C Programming
    Replies: 4
    Last Post: 06-24-2009, 01:39 PM
  5. ASCII code for a NULL character
    By GaPe in forum C Programming
    Replies: 1
    Last Post: 12-09-2001, 05:40 AM

Tags for this Thread