[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-tech
Subject:    Re: ufs free()
From:       Christian Weisgerber <naddy () mips ! inka ! de>
Date:       2018-03-31 21:15:35
Message-ID: 20180331211535.GA51168 () lorvorc ! mips ! inka ! de
[Download RAW message or body]

David Hill:

> This diff adds sizes to free(), which completes ufs/ffs.

It's broken at least for softdep+UFS2.  This chunk blows up:

> --- ufs/ffs/ffs_softdep.c	10 Feb 2018 05:24:23 -0000	1.138
> +++ ufs/ffs/ffs_softdep.c	29 Mar 2018 02:55:37 -0000
> @@ -4034,7 +4036,8 @@ handle_written_inodeblock(struct inodede
>  			*dp1 = *inodedep->id_savedino1;
>  		else
>  			*dp2 = *inodedep->id_savedino2;
> -		free(inodedep->id_savedino1, M_INODEDEP, 0);
> +		free(inodedep->id_savedino1, M_INODEDEP,
> +		    sizeof(struct ufs1_dinode));
>  		inodedep->id_savedino1 = NULL;
>  		if ((bp->b_flags & B_DELWRI) == 0)
>  			stat_inode_bitmap++;

panic: free: size too small 128 <= 256 / 2 (0xffff80000099a700) type inodedep
Stopped at      db_enter+0x5:   popq    %rbp
    TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
db_enter() at db_enter+0x5
panic() at panic+0x129
free(ffffff07259d97c8,ffffff07259d97c8,ffffff071c6d4ef8) at free+0x35b
handle_written_inodeblock(ffffff07259d97c8,ffffff071c6d4e00) at handle_written_
inodeblock+0x1b5
softdep_disk_write_complete(ffffff071c6d4e00) at softdep_disk_write_complete+0x
255
biodone(ffffff071bd999e8) at biodone+0x73

id_savedino1 and id_savedino2 are a union, so it's pretty clear
that the free() needs to distinguish between the UFS1 and UFS2 cases
like the preceding lines do.

I don't know if check_inode_unwritten() needs to make the same
distinction:

> @@ -2307,7 +2307,8 @@ check_inode_unwritten(struct inodedep *i
>  	if (inodedep->id_state & ONWORKLIST)
>  		WORKLIST_REMOVE(&inodedep->id_list);
>  	if (inodedep->id_savedino1 != NULL) {
> -		free(inodedep->id_savedino1, M_INODEDEP, 0);
> +		free(inodedep->id_savedino1, M_INODEDEP,
> +		    sizeof(struct ufs1_dinode));
>  		inodedep->id_savedino1 = NULL;
>  	}
>  	if (free_inodedep(inodedep) == 0) {

-- 
Christian "naddy" Weisgerber                          naddy@mips.inka.de

[prev in list] [next in list] [prev in thread] [next in thread]