Skip to contentSkip to conversation

Add IRQF_SAMPLE_RANDOM Flag to forcedeth

PATCH In review Patrick Simmons 4 participants 5 messages Oct 9, 2010 – Oct 10, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add IRQF_SAMPLE_RANDOM Flag to forcedeth

This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver, allowing the interrupt timing for forcedeth to be used for entropy generation. This should help /dev/random generate more secure random numbers on machines using this driver.

Please CC me with any comments as I am not subscribed to the list.

drivers/net/forcedeth.c 2010-10-09 17:14:44.880000015 -0600 renamed from drivers/net/forcedeth.c.orig 2010-10-09 17:12:08.400000015 -0600 +0 -0
@@ -3819,7 +3819,7 @@ static int nv_request_irq(struct net_dev
                 /* Request irq for rx handling */
                 sprintf(np->name_rx, "%s-rx", dev->name);
                 if 

(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_RX].vector,
- nv_nic_irq_rx, IRQF_SHARED, np->name_rx, dev) != 0) {
+ nv_nic_irq_rx, IRQF_SHARED | IRQF_SAMPLE_RANDOM, np->name_rx, dev) != 0) {
printk(KERN_INFO "forcedeth: request_irq failed

for rx %d\n", ret);
                     pci_disable_msix(np->pci_dev);
                     np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3828,7 +3828,7 @@ static int nv_request_irq(struct net_dev
                 /* Request irq for tx handling */
                 sprintf(np->name_tx, "%s-tx", dev->name);
                 if 

(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_TX].vector,
- nv_nic_irq_tx, IRQF_SHARED, np->name_tx, dev) != 0) {
+ nv_nic_irq_tx, IRQF_SHARED | IRQF_SAMPLE_RANDOM, np->name_tx, dev) != 0) {
printk(KERN_INFO "forcedeth: request_irq failed

for tx %d\n", ret);
                     pci_disable_msix(np->pci_dev);
                     np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3837,7 +3837,7 @@ static int nv_request_irq(struct net_dev
                 /* Request irq for link and timer handling */
                 sprintf(np->name_other, "%s-other", dev->name);
                 if 

(request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_OTHER].vector,
- nv_nic_irq_other, IRQF_SHARED, np->name_other, dev) != 0) {
+ nv_nic_irq_other, IRQF_SHARED | IRQF_SAMPLE_RANDOM, np->name_other, dev) != 0) {
printk(KERN_INFO "forcedeth: request_irq failed

for link %d\n", ret);
                     pci_disable_msix(np->pci_dev);
                     np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3851,7 +3851,7 @@ static int nv_request_irq(struct net_dev
                 set_msix_vector_map(dev, NV_MSI_X_VECTOR_OTHER, 
NVREG_IRQ_OTHER);
             } else {
                 /* Request irq for all interrupts */
-                if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector, handler, IRQF_SHARED, dev->name, dev) != 0) {
+                if (request_irq(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector, handler, IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev) != 0) {
                     printk(KERN_INFO "forcedeth: request_irq failed 
%d\n", ret);
                     pci_disable_msix(np->pci_dev);
                     np->msi_flags &= ~NV_MSI_X_ENABLED;
@@ -3868,7 +3868,7 @@ static int nv_request_irq(struct net_dev
         if ((ret = pci_enable_msi(np->pci_dev)) == 0) {
             np->msi_flags |= NV_MSI_ENABLED;
             dev->irq = np->pci_dev->irq;
-            if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED, dev->name, dev) != 0) {
+            if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev) != 0) {
                 printk(KERN_INFO "forcedeth: request_irq failed %d\n", 
ret);
                 pci_disable_msi(np->pci_dev);
                 np->msi_flags &= ~NV_MSI_ENABLED;
@@ -3884,7 +3884,7 @@ static int nv_request_irq(struct net_dev
         }
     }
     if (ret != 0) {
-        if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED, dev->name, dev) != 0)
+        if (request_irq(np->pci_dev->irq, handler, IRQF_SHARED | IRQF_SAMPLE_RANDOM, dev->name, dev) != 0)
             goto out_err;

     }
#1 lore raw
Quoted 4 lines · Patrick Simmons

This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver, allowing the interrupt timing for forcedeth to be used for entropy generation. This should help /dev/random generate more secure random numbers on machines using this driver.

[...]

We don't enable this for network drivers any more because:

1. At high packet rates, interrupt moderation makes interrupts very regular.
2. At low packet rates, a malicious sender can control the interrupt timing.

Ben.

Signature · 3 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
maintainer
#2 lore raw

From: Ben Hutchings <[email protected]>
Date: Sun, 10 Oct 2010 02:09:24 +0100

Quoted 5 lines
Quoted 4 lines · Patrick Simmons

This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver, allowing the interrupt timing for forcedeth to be used for entropy generation. This should help /dev/random generate more secure random numbers on machines using this driver.

Quoted 8 lines

[...]

We don't enable this for network drivers any more because:

1. At high packet rates, interrupt moderation makes interrupts very regular.
2. At low packet rates, a malicious sender can control the interrupt timing.

Agreed on all counts, I'm not applying this patch.

#3 lore raw
Quoted 10 lines · David Miller

From: Ben Hutchings<[email protected]>
Date: Sun, 10 Oct 2010 02:09:24 +0100

Quoted 7 lines
Quoted 4 lines · Patrick Simmons

This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver, allowing the interrupt timing for forcedeth to be used for entropy generation. This should help /dev/random generate more secure random numbers on machines using this driver.

[...]

Quoted 8 lines
Quoted 6 lines

We don't enable this for network drivers any more because:

1. At high packet rates, interrupt moderation makes interrupts very regular.
2. At low packet rates, a malicious sender can control the interrupt timing.

Agreed on all counts, I'm not applying this patch.

It's enabled for other network drivers, which is where I got the idea from. Has anyone actually done an experiment to see whether these two concerns are valid?

--Patrick

Signature · 1 line
If I'm not here, I've gone out to find myself.  If I get back before I return, please keep me here.
#4 lore raw
Quoted 18 lines · Patrick Simmons
Quoted 17 lines · David Miller

From: Ben Hutchings<[email protected]>
Date: Sun, 10 Oct 2010 02:09:24 +0100

Quoted 13 lines
Quoted 4 lines · Patrick Simmons

This patch adds the IRQF_SAMPLE_RANDOM flag to the forcedeth driver, allowing the interrupt timing for forcedeth to be used for entropy generation. This should help /dev/random generate more secure random numbers on machines using this driver.

[...]

We don't enable this for network drivers any more because:

1. At high packet rates, interrupt moderation makes interrupts very regular.
2. At low packet rates, a malicious sender can control the interrupt timing.

Quoted 5 lines
Quoted 1 line

Agreed on all counts, I'm not applying this patch.

It's enabled for other network drivers, which is where I got the idea from. Has anyone actually done an experiment to see whether these two concerns are valid?

Several attemps in the past tried to go into one direction or another

(Add the flag to some driver, then remove it from others)

Please read commit 9d9b8fb0e5ebf4b0398e579
http://lkml.org/lkml/2009/4/6/283

A third reason not adding is : At moderate packet rates, _no_ entropy is feeded at all because add_interrupt_randomness()/add_timer_randomness is _very_ conservative, with first, second-order and third-order estimates.

credit_entropy_bits() is called with 0 bit

Adding this stuff has a high cost, I can see it in profiles on machines with tg3 nics. I often remove the IRQF_SAMPLE_RANDOM flag localy.