| Message ID | 20230205042951.3570008-5-alxndr@bu.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Retire Fork-Based Fuzzing | expand |
On 5/2/23 05:29, Alexander Bulekov wrote: > As we have repplaced fork-based fuzzing, with reboots - we can no longer Typo "replaced". > use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer > that it uses to catch slow inputs, however these timeouts are usually > seconds-minutes long: more than enough to bog-down the fuzzing process. > However, I found that slow inputs often attempt to fill overly large DMA > requests. Thus, we can mitigate most timeouts by setting a cap on the > total number of DMA bytes written by an input. > > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> > --- > tests/qtest/fuzz/generic_fuzz.c | 5 +++++ > 1 file changed, 5 insertions(+) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Hi Alex, On Saturday, 2023-02-04 at 23:29:45 -05, Alexander Bulekov wrote: > As we have repplaced fork-based fuzzing, with reboots - we can no longer > use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer > that it uses to catch slow inputs, however these timeouts are usually > seconds-minutes long: more than enough to bog-down the fuzzing process. > However, I found that slow inputs often attempt to fill overly large DMA > requests. Thus, we can mitigate most timeouts by setting a cap on the > total number of DMA bytes written by an input. > > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> > --- > tests/qtest/fuzz/generic_fuzz.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c > index c2e5642150..eab92cbc23 100644 > --- a/tests/qtest/fuzz/generic_fuzz.c > +++ b/tests/qtest/fuzz/generic_fuzz.c > @@ -52,6 +52,7 @@ enum cmds { > #define USEC_IN_SEC 1000000000 > > #define MAX_DMA_FILL_SIZE 0x10000 > +#define MAX_TOTAL_DMA_SIZE 0x10000000 > > #define PCI_HOST_BRIDGE_CFG 0xcf8 > #define PCI_HOST_BRIDGE_DATA 0xcfc > @@ -64,6 +65,7 @@ typedef struct { > static useconds_t timeout = DEFAULT_TIMEOUT_US; > > static bool qtest_log_enabled; > +size_t dma_bytes_written; > > MemoryRegion *sparse_mem_mr; > > @@ -197,6 +199,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr) > */ > if (dma_patterns->len == 0 > || len == 0 > + || dma_bytes_written > MAX_TOTAL_DMA_SIZE NIT: Just wondering if you should check dma_bytes_written + l as opposed to dma_bytes_written? It's probably not important enough given it's just an artificial limit, but thought I'd ask. > || (mr != current_machine->ram && mr != sparse_mem_mr)) { > return; > } > @@ -269,6 +272,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr) > fflush(stderr); > } > qtest_memwrite(qts_global, addr, buf, l); > + dma_bytes_written += l; > } > len -= l; > buf += l; > @@ -648,6 +652,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) > > op_clear_dma_patterns(s, NULL, 0); > pci_disabled = false; > + dma_bytes_written = 0; > > QPCIBus *pcibus = qpci_new_pc(s, NULL); > g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus); > -- > 2.39.0 While this will still consume the existing corpus, is it likely to cause these existing corpus to be trimmed? Otherwise, the changes look good: Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Thanks, Darren.
On 230213 1438, Darren Kenny wrote: > Hi Alex, > > On Saturday, 2023-02-04 at 23:29:45 -05, Alexander Bulekov wrote: > > As we have repplaced fork-based fuzzing, with reboots - we can no longer > > use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer > > that it uses to catch slow inputs, however these timeouts are usually > > seconds-minutes long: more than enough to bog-down the fuzzing process. > > However, I found that slow inputs often attempt to fill overly large DMA > > requests. Thus, we can mitigate most timeouts by setting a cap on the > > total number of DMA bytes written by an input. > > > > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> > > --- > > tests/qtest/fuzz/generic_fuzz.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c > > index c2e5642150..eab92cbc23 100644 > > --- a/tests/qtest/fuzz/generic_fuzz.c > > +++ b/tests/qtest/fuzz/generic_fuzz.c > > @@ -52,6 +52,7 @@ enum cmds { > > #define USEC_IN_SEC 1000000000 > > > > #define MAX_DMA_FILL_SIZE 0x10000 > > +#define MAX_TOTAL_DMA_SIZE 0x10000000 > > > > #define PCI_HOST_BRIDGE_CFG 0xcf8 > > #define PCI_HOST_BRIDGE_DATA 0xcfc > > @@ -64,6 +65,7 @@ typedef struct { > > static useconds_t timeout = DEFAULT_TIMEOUT_US; > > > > static bool qtest_log_enabled; > > +size_t dma_bytes_written; > > > > MemoryRegion *sparse_mem_mr; > > > > @@ -197,6 +199,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr) > > */ > > if (dma_patterns->len == 0 > > || len == 0 > > + || dma_bytes_written > MAX_TOTAL_DMA_SIZE > > NIT: Just wondering if you should check dma_bytes_written + l as opposed > to dma_bytes_written? It's probably not important enough given it's > just an artificial limit, but thought I'd ask. > Done :) > > || (mr != current_machine->ram && mr != sparse_mem_mr)) { > > return; > > } > > @@ -269,6 +272,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr) > > fflush(stderr); > > } > > qtest_memwrite(qts_global, addr, buf, l); > > + dma_bytes_written += l; > > } > > len -= l; > > buf += l; > > @@ -648,6 +652,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) > > > > op_clear_dma_patterns(s, NULL, 0); > > pci_disabled = false; > > + dma_bytes_written = 0; > > > > QPCIBus *pcibus = qpci_new_pc(s, NULL); > > g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus); > > -- > > 2.39.0 > > While this will still consume the existing corpus, is it likely to > cause these existing corpus to be trimmed? Not sure - It would affect inputs that generate a lot of DMA activity (though those should have been caught by our previous timeout mechanism). > > Otherwise, the changes look good: > > Reviewed-by: Darren Kenny <darren.kenny@oracle.com> > > Thanks, > > Darren.
diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c index c2e5642150..eab92cbc23 100644 --- a/tests/qtest/fuzz/generic_fuzz.c +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -52,6 +52,7 @@ enum cmds { #define USEC_IN_SEC 1000000000 #define MAX_DMA_FILL_SIZE 0x10000 +#define MAX_TOTAL_DMA_SIZE 0x10000000 #define PCI_HOST_BRIDGE_CFG 0xcf8 #define PCI_HOST_BRIDGE_DATA 0xcfc @@ -64,6 +65,7 @@ typedef struct { static useconds_t timeout = DEFAULT_TIMEOUT_US; static bool qtest_log_enabled; +size_t dma_bytes_written; MemoryRegion *sparse_mem_mr; @@ -197,6 +199,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr) */ if (dma_patterns->len == 0 || len == 0 + || dma_bytes_written > MAX_TOTAL_DMA_SIZE || (mr != current_machine->ram && mr != sparse_mem_mr)) { return; } @@ -269,6 +272,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr) fflush(stderr); } qtest_memwrite(qts_global, addr, buf, l); + dma_bytes_written += l; } len -= l; buf += l; @@ -648,6 +652,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) op_clear_dma_patterns(s, NULL, 0); pci_disabled = false; + dma_bytes_written = 0; QPCIBus *pcibus = qpci_new_pc(s, NULL); g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus);
As we have repplaced fork-based fuzzing, with reboots - we can no longer use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer that it uses to catch slow inputs, however these timeouts are usually seconds-minutes long: more than enough to bog-down the fuzzing process. However, I found that slow inputs often attempt to fill overly large DMA requests. Thus, we can mitigate most timeouts by setting a cap on the total number of DMA bytes written by an input. Signed-off-by: Alexander Bulekov <alxndr@bu.edu> --- tests/qtest/fuzz/generic_fuzz.c | 5 +++++ 1 file changed, 5 insertions(+)