[Bug 228807] dump can core

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=228807

--- Comment #2 from Diane Bruce <db at FreeBSD.org> ---
> > +   sblock = NULL;                                                          
> >     if ((ret = sbget(diskfd, &sblock, -1)) != 0) {                          
> >             switch (ret) {                                                  
> >             case ENOENT:                                                    

This is all completely bogus as mentioned by @emaste. I know                    
exactly the bug now. Here are the excruciating details. (Well most of it)       
Setting sblock to NULL is not needed here because of the way sbget works.       
Heinsenberg was playing here.                                                   

dump main.c starts up and works out sizes of various things                     
looking at line 454 and 455 We see the following snippet                        
...                                                                             
maxino = sblock->fs_ipg * sblock->fs_ncg;                                       
mapsize = roundup(howmany(maxino, CHAR_BIT), TP_BSIZE);                         
...                                                                             

Looking at my SSD and using printfs in main.c we see    
sblock->fs_ipg= 80256 sblock->fs_ncg = 655 maxino=52567680                      
mapsize = 6571008                                                               

inodes are dumped to "tape" from dumpino() line 459 in traverse.c               

dumpmap is then called line 832 traverse.c                                      

On my SSD spcl.c_count is worked out in and this results in a spcl.c_count=6417 

Eventually flushtape() line 227 in tape.c is called                             

line 296 of tape.c we have                                                      

...                                                                             
        if (spcl.c_type != TS_END) {                                            
                for (i = 0; i < spcl.c_count; i++)                              
                        if (spcl.c_addr[i] != 0)                                
                                blks++;                                         
        }                                                                       
...                                                                             
spcl.c_count of 6147 is quite a bit out of bounds for spcl.c_addr               
and it segfaults here.                                                          

In fact, looking at /usr/include/protocols/dumprestore.h                        

line 117                                                                        
...                                                                             

                int32_t c_count;            /* number of valid c_addr entries
*/
                char    c_addr[TP_NINDIR];  /* 1 => data; 0 => hole in inode */ 
                char    c_label[LBLSIZE];   /* dump label */                    
...                                                                             

Looking at TP_NINDIR comment at line 53 of dumprestore.h                        

...                                                                             

* TP_NINDIR is the number of indirect pointers in a TS_INODE                    
* or TS_ADDR record. Note that it must be a power of two.                       

 */                                                                             
#define TP_BSIZE        1024                                                    
#define NTREC           10                                                      
#define HIGHDENSITYTREC 32                                                      
#define TP_NINDIR       (TP_BSIZE/2)                                            

...                                                                             

Hence c_addr[TP_NINDIR]; is 512 bytes. We overflow this fairly easily.          

&spcl = 0x2100d8                                                                
  top = 0x2104d8                                                                

&pcl.c_addr[3715]=0x210fff                                                      

(lldb) x/x 0x210ffe                                                             
0x00210ffe: 0x00000000                                                          
warning: Not all bytes (2/4) were able to be read from 0x210ffe.                

Boom.                                                                           

Apparently when the sblock buffer was static, the linked arrangment             
of sblock_buf[MAXBSIZE] and spcl from dumprestore.h was "just right"            
to mask this bounds error. This would also explain why malloc'ing               
a huge area for the superblock made no difference.                              

I don't know how to fix this. You will note that a dump on my spinning          
rust system will also go out of bounds in c_addr but perhaps the                
following variables are not needed at this point? e.g. c_label and up.          
It's also overflowing spcl itself and going into memory it shouldn't.           
I'd not trust the dump itself at this point.                                    
...                                                                             

sblock->fs_ipg= 80256 sblock->fs_ncg = 319 maxino=25601664                      
mapsize = 3201024                                                               

...                                                                             
spcl=0x2100d8                                                                   
top= 0x2104d8            
spcl.c_count=3126                                                               

spcl.c_addr[3125]=0 blks=266                                                    

 spcl.c_addr[3125]=0 blks=267                                                   
&spcl.c_addr[3125]=0x210db1                                                     

^^ it doesn't segfault here because it doesn't get to a page boundary           
but it certainly is outside the bounds of spcl.                                 

This was fun!

-- 
You are receiving this mail because:
You are the assignee for the bug.